Hacker News new | ask | show | jobs
by Dru89 3687 days ago
I'm unhappy with the approval process being a simple search for "LGTM". I wish GitHub pull requests had actual support for a review process, e.g.:

* open issues to address

* review state, such as "changes requested" or "approved" (along with users that are in each state).

We've been using Phabricator's[1] Differential tool for code reviews and it feels superior to this process, but it would certainly be nice to have an all-encompassing solution for this.

[1] http://phabricator.org

2 comments

An actual flag wouldn't be bad, but I know that I've sometimes given out conditional approvals in a code review (i.e., "if you change this, then I approve; if you don't agree, then we should talk"). The idea being to remove a round-trip that would otherwise waste the reviewee's time. (This of course implies a certain degree of trust that the reviewee makes the change as you desire, but in practice I find this isn't a problem.)
Whenever I'm working on a big patchset, I always make sure I add a checklist. Unfortunately nothing forces me to do this, so some people make huge patchsets and there's no history of what was changed or fixed in that patchset. This makes review and maintainence quite frustrating.