[dmd-beta] Cherry-picking II

[Brad Roberts]

On 1/29/14, 12:07 AM, Andrew Edwards wrote:
> On 1/28/14, 1:04 PM, Brad Roberts wrote:
>> IMHO, a much more workable solution is to use pull requests just like
>> for any other branch.  If someone is requesting a merge to a release
>> branch, then they should assemble the pull request and submit it.  If
>> you are deciding a fix should be merged to the release branch, put
>> together the pull request just like anyone else would.  That gains
>> several advantages:
>>
>>   1) gives a good chance to review exactly what changes are going to
>> be made
>>   2) gives the auto-tester a chance to validate then changes
>>   3) gives a chance for additional eyeballs to be watching for mistakes
>>
>> The only con is that it's more steps, but without those steps, the
>> gains aren't possible.  For any regular developer, putting together a
>> pull request is something they can do in their sleep, so the cost is
>> pretty small.
>>
> I'm not necessarily against this but I have a few questions.
>
> 1) A change is placed in git-hub and reviewed prior to being merged into
> master. Without such a review it will not be accepted. Why now should we
> hold another review session prior to picking something to include into
> the branch? Isn't it better to require a minimum number of reviewers
> (say three for good measures) to approve a change before to committing
> to master? That way auto designation of such changes can be made at the
> time of review with a majority vote from the reviewers.

Not at all.  As a release branch progresses, it naturally grows further 
out of sync with the master branch.  That by itself lends to some 
justification for a re-review, even if a very quick on.  Additionally, 
it's a sanity check to allow for the opportunity to make sure what's 
being merged is what's intended to be merged.  Pushing directly into 
_any_ branch rather than going through a merge pull raises the risk of 
accidents happening.  Such accidents have happened more than a few times 
throughout our history.  Even more have happened and caused no damage in 
wrongly constructed pull requests, indicating the value of stepping 
through a pull request and avoiding damaging the master branch.  It has 
much less to do with correctness of the change, but rather with 
validation of the merge and it's continued correctness within the 
differing code base.

> 2) We just agreed upon a naming scheme that you insisted had to meet a
> certain convention in order to guarantee validation by the auto-tester.

Not relevant.

> If the auto tester already test this branch, and it does, why now would
> I need another way of monitoring what changes made to the branch?

See above.

> 3) How often have you seen a request for comment on a particular pull go
> unanswered? Just this past week, several request were made by Daniel
> Murphy than no on responded to. In the end, he made the decision on his
> own. Here are a couple that still remain unanswered:
>
> https://github.com/D-Programming-Language/dmd/pull/3120#issuecomment-33344986
> https://github.com/D-Programming-Language/dmd/pull/3118#issuecomment-33309502
> https://github.com/D-Programming-Language/phobos/pull/1864#issuecomment-32484778

Don't conflate the multitude of open pull requests to master with the 
tiny number of pull requests that are likely to ever be opened against 
the release branches.  The usage patterns, while similar, aren't the 
same.  Additionally, our processes are evolving.  One instance of it not 
going well has only minor bearing on continuing to seek one that 
balances correctness, safety, and ease of practice.

> By doing this we would be unnecessarily inducing another delay in the
> process: which is counterproductive.

I don't believe that's accurate.  What I suggest is a substitution of a 
request for you to merge that comes in via a bug notation, an email, or 
a pull request note in the master branch pull with a pull request.  The 
latter being significantly less likely to get lost in the noise than 
most of the former.  Lastly, by constructing and submitting a pull 
request, chances are even higher that by the time you see it, it's 
already both been validated by the requester as matching his intent and 
by the auto-tester as passing tests.  That just leaves a little button 
pressing to perform the merge.


Anyway, those are my 2 cents and they're certainly not the only way of 
handling things, just the way I would.

Later,
Brad