Dreamwidth Development

There are two ways to leave per-line comments when reviewing a pull request:

• opening up each commit in a separate tab and going through them one-by-one (commits are listed in both the Discussion and the Commits tabs)

• going to the Files Changed tab, and seeing the entire final result

I used to open up each commit individually, but:

• if the person is forced to rebase (either to get rid of extra commits, or to clean up their history), the discussion is lost

• under the discussions tab, there's no context, just the comment, which makes it possible to know that there's activity going on, but very difficult to keep track of exact context

• it's possible that something you reviewed in one commit was fixed later on in the commit series, invalidating your comment

• in the email notification, the "view it on github" link shows you the commit, not the associated pull request (and there's no easy way to find which pull request a commit is on, given just the commit)

I'd been ignoring the Files Changed tab for some reason, but I've now found that they actually don't have any the disadvantages of the per-commit view:

• if the person is forced to rebase, old discussion is preserved in the pull request. If the rebase altered the diff, you'll get an "comment made on outdated diff"; if the diff is the same after a rebase, it shows up like it did before

• under the discussions tab, it shows the context of the diff you're commenting on, along with your comment, which makes it very easy to keep track of the exact reason a comment was made (especially on short comments hehe)

• you see the final version of the code that's being submitted

• in the email notification, "view it on github" links to the pull request where all the discussion is taking place

All this is to say that if you're poking around and want to comment on something you see in someone else's pull request (and we do encourage code review in this manner!), use the Files Changed tab. It's much more useful.

Some examples:

• actual pull request where you can see how easy it is to tell that a review comment has been handled (and what still needs work, or what doesn't need to be changed after all)

• test pull request where I committed a change, commented using both the commit/files changed view, rebased+force pushed the rebase (which blew away the comment on the commit, but retained the comment on files changed), and then made the diff outdated -- which collapsed the discussion but still kept it visible