Hi Chuck,
Thanks for approving the patch.
Chuck Bell wrote:
> STATUS
> ------
> Patch approved pending completion of requests.
>
> REQUESTS
> --------
> 1. The backup_errors test fails on Windows.
The problem you reported is most probably due to the fact that you have not
succeed in patching errmsg.txt. Test result contains numeric error codes because
there is missing --replace_column in front of one of SHOW WARNINGS statements
(which was the case even before my patch). I can fix it when pushing my patch.
> 2. The patch fails to apply to latest tree. Please resolve as the
> kernel.cc code has changed a bit.
>
> Hunk #13 succeeded at 1303 (offset 3 lines).
> Hunk #14 FAILED at 2027.
> 3 out of 14 hunks FAILED -- saving rejects to file sql/backup/kernel.cc.rej
> patching file sql/backup/logger.h
> patching file sql/share/errmsg.txt
> Hunk #1 FAILED at 6442.
> 1 out of 1 hunk FAILED -- saving rejects to file sql/share/errmsg.txt.rej
>
You can not expect that the patch you are reviewing will always cleanly apply to
the most recent tree. Usually some time passes between the moment the patch is
submitted and the moment you start reviewing it. In this time someone can push
new code which will conflict with the patch you are reviewing.
Resolving such conflicts is part of the push procedure after the patch has been
approved. Sometimes it might require big changes to the patch, in which case
reviewers should be consulted, I agree. But in most cases it is a simple merge
and I think we can trust each other that we will do it correctly.
To help in applying the patch for review purposes, in future I'll try to include
revision-id of the tree against which the patch was prepared in the comments.
For this patch the id is:
ingo.struewing@stripped
Rafal