|
|
|
|
|
by da39a3ee
998 days ago
|
|
> I'm sure I'm wrong about some detail about some of the points above. Someone is likely to say "he could've just done this to solve problem 5(a)" Yep, I'm going to do that! > Work-in-progress commits towards addressing review feedback become visible as soon as the branch is pushed. This forces contributors to address all feedback in a single commit, or for reviewers to deal with partially-addressed feedback. This point isn't really valid. It assumes that the contributor wants to push their new work addressing feedback to the remote as soon as they make each commit. That's OK, so do I (in case my laptop disappears or whatever). But, it takes 1 second to make a git branch, so you can just push your new commits there and then merge or cherry-pick onto the PR branch when you're ready. |
|
To review I like branches. Bad commits are not pleasant to look at, but they carry information, too, e.g., revealing the need for discussion. It's a bit like asynchronous pairing.
I don't think looking at the code on a website is sufficient as review in many cases, so I typically have a copy of the branch anyway. If any feature is needed, it is the possibiliy to comment on unchanged lines.
It probably depends on the scope. If the reviewer works on a more abstract level and can rely on authors to get the details right, maybe github is not ideal. Where the reviewer is almost as deeply involved as the author, a branch works nicely.