Reviews are hard

It’s a vast subject, but one thing is certain: reviewing other people’s code is hard. Because good mentoring require technical and non-technical skills (such as patience).

I would like to dive directly into a specific detail of code reviews. It’s an iterative process: author submits code for review, reviewer make suggestions, author amends or pushes more code, reviewer make different or more suggestions, and so on.

In Git, « more code » takes the form or one or more commits appended to the Pull Request (or Merge Request if you use Gitlab, for simplicity I’ll just use « Pull Request » in this piece). And « amended code » means overwriting existing commits and force-pushing, which makes the old commits disappear.

As a reviewer, it can be very annoying because what I first look for in an update is whether my suggestions have been implemented or not, and how. That’s why authors are sometimes encouraged to push new commits in their Pull Requests and never overwrite existing ones. It makes the reviewer’s job way easier, because the UI can just show the new commit and they’ll know what’s changed.

But having this policy has drawbacks. When the Pull Request is merged (by fast-forward or not) in can leave awkward commits in the history, like « implement suggestions », « fix according to review », « review fixes again », etc… And merging the PR by squashing it isn’t always relevant, sometimes I do want to have several commits, because they address different parts of the problem.

How can we solve this? Well, it would be nice if I could see the difference between the current state and the last time I reviewed regardless of whether the author has amended their commit or not. And for that, I need to have a local copy of that commit. Fortunately that’s one of the things that git is very good at: you just make a local branch that tracks the PR’s branch, and when the code is changed you make another one. And then you can diff between those branches and see what changed.

OK, sounds simple enough. I have an itch, let me scratch it.

I present you git-pr-branch! A « small » utility that will create branches from Pull Requests, and do a few things with them. You’ll be able to automatically create the PR-based branches that I just explained about. You’ll also be able to display a nice listing of the branches, their associated PR, the PR status (open or closed), and the PR URL to clickety click. And since this can end up in quite a lot of branches, there’s also a sub-command to clean all that up, and delete branches whose PR is closed.

Ironically, it’s hosted on Gitlab but at the moment it only works on Github and Pagure. I’ll add Gitlab support if I end up working more with Gitlab (something tells me it’s likely to happen in the near future), but you can also send me a patch if you want it sooner.

While writing this side project, I discovered the fantastic python library attrs. It’s really awesome, I encourage you to try it out. (as always, my side projects are a good opportunity to try out new libraries or frameworks that I discover 😉 )

The python packages are on PyPI, and for the lazy Fedora and Mageia users out there I’ve made a COPR repository that you can enable on Fedora 32 and Mageia 7. Once installed, just run git-pr-branch (or git pr-branch) to discover the available commands.

Feel free to tell me what you think of the tool. Do you like the idea? Are you going to use it in your reviewer workflow? Did I bother writing code again when there is an obvious and better tool to do it? Let me know! 🙂

[EDIT] I’ve made a COPR repo, added the link.
[EDIT2] It now works with Pagure too, added the reference.


Publié

dans

, ,

par

Étiquettes :