From a diary of AArch64 porter — handling big patches

This post is part 12 of the "From a diary of AArch64 porter" series:

There are moments when developers want to make changes in other FOSS projects. Which usually means dealing with patch reviews. On mailing list, on Gerrit, GitHub or any other collaboration platform etc. And it can take quite a long time.

One note at start: I am giving code patches as examples but post applies to any type of contributions (code, docs, examples, tests etc.).

What affects review

How quickly patch (or patchset) gets reviewed depends on many things:

size of patch
how complicate patch is
size of development team
how busy core developers are

Usually we do not have a way to address last two entries but can work on first two ones.

How to make patch easier to review

There are several things you can do to make patch easier to review. Let me try share some hints.

Write cover letter

Does not matter are you going to send one patch or whole set you need to write why your contribution matters. Which features it adds/fixes/removes and why there is a need for it, why the way you wrote it is good etc.

Let reviewers know why they have to spend time on it.

There is the great article by Chris Beams “How to Write a Git Commit Message“. Read it and edit your patch description if needed.

Split into smaller parts

Look at your patch and check is it self-contained change.

No one wants to review 130 kilobytes of text at one time. I had such patch when was dealing with adding AArch64 support to Qt in past.

In such case you need to look at patch and start splitting it into smaller pieces. If you do not know how then ask upstream developers to take a look at diffstat output and give hints. For my Qt work it was quick “this path goes to this component, that path to that component” instruction. I managed to cut one patch into series and get it reviewed (after changes upstream merged them).

Other occasion was adding non-x86 support into OpenStack Kolla. Patch had 50 revisions before merge. In meantime I wrote at least 20 other patches to make it possible and several of them I split from main one.

So take a look at your patch and think do you need to split it or not.

Moving existing code

If you need to rename some files in repository and do changes to them then split patch into parts:

rename files
do changes

This way upstream developers can look at first patch and accept as there are no code changes. And then look at following patch(es).

Note that it does not need to be a file rename even. If you refactor existing code you may want to move some classes/functions/defines to separate files. Just remember: one patch moves, second does changes.

Reacting to review

Sooner or later someone will look at your patch and leave comments. Read them, reply, explain. Do requested changes or write why you think they are not needed or why the other way would be even better.

Remember that other developers are human too — be gentle. No one likes angry reactions (if you need to shout out your anger then do it offline). Acting badly may mark your patch as “ignore that $#$@% author”. As a result you may need more time to get your contribution merged.

When to rebase

Often your patch stays in review queue while project moves on. In such case check for moments when you need to rebase your patch on top of current development. Especially when your changes are in merge conflict with up-to-date source.

Avoid doing it too often — at some projects if patch is not in merge conflict then it is fine even if it is days old.

Useful tools

There are many tools handy when dealing with patches. Get familiar with diffstat and git diff. Years ago I would recommend quilt but most people just use git instead.

If you are dealing with really big patches then try Midnight Commander (mc). It allows to enter patch the same way it does with directories or archives. You may even delete parts of patch (just remember to refresh patch to get rid of empty changes).

Also check what kind of help you can get from your code editor. Nowadays most of programmers’ editors have a way to add extensions, scripts, plugins etc.

Summary

Do not be afraid of patch reviews. Both as author of a change and also as contributor or developer. It is good to check how project does patch reviews and even taking a part.

Spend a bit of time on your patch before sending it out. Do a break, read it again etc. It will help both sides.