[phobos] Friendly reminder about pull requests

[Jonathan M Davis]

On Monday, July 02, 2012 01:11:31 Andrei Alexandrescu wrote:
> On 7/1/12 11:30 PM, Jonathan M Davis wrote:
> > Well, the diff that you see when you click on diff in the pull request
> > gives a diff in comparison to the commits prior to the changes being
> > committed.  So, what you're looking for is being able to diff the state
> > of all of Phobos in the pull request against the state of all of Phobos
> > in the current master?
> Yes. Say I enter a pull request that has been already discussed and
> modified as a result of the discussion. From here there are two paths of
> analyzing the request:
> 
> 1. Going through all comments and reactions to them, keeping a mental
> track of the evolution of the code;
> 
> 2. Reading the comments only for validity and then look at the
> "integral" - the pull request as it is being proposed right now, against
> the existing code.
> 
> I very much prefer (2) perhaps because phabricator.org used me to it. In
> unusually large pull requests I might occasionally care for how the
> proponent responded to a particular comment. But generally I want to see
> what's there now and what's being proposed, wholesale.

Well, if the request is rebased on master, then that fixes the problem, but if 
you want to compare agains the current master, that's up to you.

> > I believe that this explains how to do it:
> > 
> > https://github.com/blog/683-cross-repository-compare-view
> > 
> > Though I don't see how to actually _get_ to the compare view that they're
> > describing without typing in the URL. For Kenji's repository, you'd type
> > 
> > https://github.com/9rnsr/phobos/compare/master
> > 
> > For mine you'd type
> > 
> > https://github.com/jmdavis/phobos/compare/master
> > 
> > and then you can manipulate the repository and branches being compared
> > using the compare view.
> > 
> > Personally, I think that seeing all of that extra information is just
> > distracting though.
> 
> I don't see that information as not extra, but instead exactly what the
> code will be after the merge.

When it includes _all_ of the changes in Phobos since the brach was merged, 
that can be a lot of extraneous stuff. And the longer that a pull request has 
been around without merging in or rebasing on master, the worse that it gets. 
But if you don't mind, then that's fine. I just find it to be too much to look 
at personally.

> I think I found what I want. At the bottom of the request there's this:
> "Tip: You can also add notes to lines changed in a file under Diff". The
> word "Diff" is linked, and the link seems to point to what I need.

Cool. That'll make it considerably easier.

- Jonathan M Davis