On 6/8/15 10:17 AM, William A Rowe Jr wrote:
> In this example, the patch was enhanced and the original reviewers' efforts
> were thrown away. It's a shame to waste the limited review cycles.
>
> Moving forwards, can we please do two things. 1) retain the original patch
> and
> vote in the STATUS, adding a revised patch below the vote that can be
> reviewed,
> and 2) show the changelog of patch 1 vs. patch 2 so that the original reviewer
> doesn't have to bisect two patches to determine whether the change had any
> measurable effect on the original patch review?
>
> We might start an
> http://svn.apache.org/repos/asf/httpd/httpd/patches/backports/2.[24].x/ tree,
> so that viewvc can be helpful in this respect, instead of multiple patches
> residing in our individual public_html/ trees. WDYAT?
>
> All of this is to say that we should pay extra close attention to correctness
> when we first submit a backport patch proposal, and request 3) is to please
> not
> use STATUS as a discussion tool. If the comment/observation doesn't fit in
> 240
> characters (3 lines), it probably belongs on the list instead, or can be a
> short note in STATUS with a long explanation on dev.
This would be a lot less painful if the httpd project would actually use SVN
for managing these patches. The the SVN project does this. Here's an example
of a SVN STATUS entry (from the 1.8.x release branch):
[[[
* r1654932, r1654933, r1654934, r1654937
Fix issue #4554, "0 file length reported in FSFS".
Justification:
This causes 'svnadmin dump' to create corrupted output that fails to
load and we provide no way to detect that problem other than loading
the respective dump. We also want to prevent further instances of
that issue to be added to the repository.
Notes:
r1561419 has been removed from this change set and proposed for a
separate backport because it is not a necessary part of the #4554 fix.
Branch:
^/subversion/branches/1.8.x-issue4554-v2
Votes:
+1: rhuijben
+1: stefan2 (before -v2 branch)
+0: danielsh (hard to review all potential causes of expanded_size==0 in
7*3*2 cases (1.1…1.8) × (file/dir/prop rep) × (plain/delta))
]]]
The first line is the trunk revisions being backported. However, these changes
do not cleanly merge from trunk onto 1.8.x. So a backport branch was made.
The change was merged and any conflicts resolved (or possibly other changes).
This particular change is a tad unusual in that a second backport branch was
made. Usually any modifications just mean additional changes on the branch and
the vote split would notice which branch changes weren't included in the review.
The workflow for the httpd project would look something like this (presuming
you start with the 2.4.x branch checked out in your $PWD):
svn cp ^/httpd/httpd/branches/2.4.x
^/httpd/httpd/branches/2.4.x-ap-listeners-buckets
svn switch ^/httpd/httpd/branches/2.4.x-ap-listeners-buckets
svn merge -c 'r1640763, r1643179, r1656368' ^/httpd/httpd/trunk .
# handle conflicts or anything needed.
svn ci
# Further changes repeat the merge/commit.
svn switch ^/httpd/httpd/branches/2.4.x
Then when approved you merge the branch onto the release branch like so onto
the 2.4.x branch (presuming you start with 2.4.x branch checked out in your
$PWD):
svn merge ^/httpd/httpd/branches/2.4.x-ap-listeners-buckets .
svn rm ^/httpd/httpd/branches/2.4.x-ap-listeners-buckets
Using this workflow resolves almost all of your requests.
1) You can preserve votes easily by noting when someone didn't vote for new
revisions on the branch and moving their vote to a separate line.
2) You have history, no need to try and track the history in the STATUS file.
3) This doesn't resolve this issue and the SVN project somewhat does the same
as you can see above. But I think it helps because it's far less confusing
about what everyone reviewed and what their comments are about.
