Hacker News new | ask | show | jobs
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.

3 comments

If an author is unable to produce readable commits on the branch of a PR the author is likely not able to produce readable change-sets either.

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.

I did wonder about that when I read it. The idea of changesets and versions sounds an awful lot like branches and commits.
Yeah, that one baffled me, too. The rest makes sense, though!