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
_______________________________________________
dmd-beta mailing list
[email protected]
http://lists.puremagic.com/mailman/listinfo/dmd-beta
