How to make Pull Requests great for informal teams
Recently GitHub is changing Pull Requests, adding new features to it to ease collaboration. However it seems to improve only a certain kind of collaboration; a formal one. When you work in a more informal way with your friends or colleagues, these new features can get in the way.
The newest change to Pull Requests, allowing to dismiss the formal review, shows to me the bad design behind the new Pull Request Reviews.
Not without reason
Don't get me wrong, it is not bad GitHub is changing the Pull Requests. There are good reasons to do so.
- It was often unclear whether a comment was something you had to solve or just a minor improvement.
- Discussions were hidden even though the underlying problem was not fixed yet. Creating annoying comments like "discussion x is still open".
- Or the other way around, they were kept open when the problem was fixed, because in the end, the line commented on was not part of the fix.
There were probably more issues with the tool set in Pull Requests, but these were the ones I crossed in the teams I work with.
There's at least one more reason I can see for more formal review processes: as the maintainer of a repository there was no helping hand in communicating if a PR was approved or needed changes.
Enter the new tool set
The latest changes made by GitHub try to solve these problems. However, to my eye, they have a strong emphasis on formal review processes. I.e. enterprises or open source maintainer — fan relations.
It allows to easily show the vote of the reviewer or maintainer of a project. Good for a more formal open source relation.
And, more enterprisy, it allows to block merging before a review is deemed correct by the reviewer.
Apparently, this didn't always work for everyone.
Today GitHub added a way to circumvent reviews blocking a merge. According to their announcement for cases where the reviewer "goes on vacation or runs into computer problems". I would say, it was also requested by teams with more informal review processes. However, it only helps so-so.
Old problems out, new problems in
Because with the old problems maybe gone, new ones come in.
- Now discussions in which the problem was fixed are kept open, forever. Where before they were sometimes closed primarily or kept open in edge-cases, now it is completely impossible to see which discussions still need attention: they are all open.
- Comments are not shared directly making it hard to work together. Especially on large PRs, or when heaving a tight deadline, it is great to ping-pong comments; the reviewer adding line comments, the author fixing them as they come in. However, line comments are not visible for others anymore, until you press the extra 'review changes' button and give a verdict.
- Even worse, I've seen people forget that extra button and not knowing that their carefully crafted line comments aren't send to the author.
- Multiple comments don't always share the same conclusion. Some comments might be positive, other might require a change. You could start two different reviews, but that's breaking your line of thought and how you go through the diff.
- And maybe minor, but I've seen cases where the review summary is looked over by people as a separate comment needing attention.
Improvements for informal teams …
I think there are ways to improve this. Or at least, for the teams I know.
I also think that the previous interface of Pull Requests wasn't that bad. All I wish for are a few small changes.
- Close line comments as solved, or re-open them if they are hidden.
This allows to keep a good overview of still active discussions. It allows to close discussions if they are solved, but also if a question is answered. And if a line comment is automatically hidden, to (re-)open it if the problem isn’t solved. This way, you end up with a stream of comments which clearly show the parts needing attention. - Show approval or disapproval with a normal comment.
This allows to use the currently new review summary, from the discussion view, without having to go to the diff view.
That might sound weird — not seeing the diff before (dis)approving. However, when you as a reviewer come back to a second time, you can already easily see the new changes from the discussion view. I often use that to check those comments, and then when back in discussion view, give my final remark. - Make the combination of line comments and approval optional.
This one is for keyboard-lovers. With the new features also came the double submit button at line comments. This means using ctrl/cmd + enter uses the new way in which a review summary is needed. Making that a choice (in the project or user settings) gives a better work-flow for people preferring old-style line comments.
… and formal teams
Formal teams can still use the new feature set. And also benefit the additions for more informal teams. I.e. closing discussions, giving overview in the discussion streams, help teams whatever work flow.
I guess both tool sets can happily live side by side.
Onwards
A fan
