Grokking OpenStack

OpenStack - little pieces

Reviewing an OpenStack Patch

Every OpenStack contributor wants their patches reviewed and I am the same. Getting good feedback or an approval helps to achieve the mission the patch was created for in the first place, to add a feature or fix a bug. One of the best ways to get your patch reviewed is by reviewing patches submitted by others. People recognize the name of their patch reviewers and are appreciative. If people know you, your submitted patch might get their attention faster than if they don’t know you. Providing good reviews for patches also helps you to write good patches with concise patch titles and accurate descriptions, all part of a good patch.

Let’s take a look at a few points that might help you get started reviewing patches.

First of all, you will never know enough to totally feel confident before you begin reviewing patches, you just have to do it. If you are waiting to feel confident before you begin, you will never get the practice you need to feel confident.

Anyone who is signed in to review.openstack.org can review a patch.

If you have received reviews in the past on your own patches, consider the most helpful comments you received and try to offer the same kind of support to the creator of patches you review. All of us are learning, and helpful review comments can really provide a lot of guidance for patch composers as they work to create an effective patch.

To get started, select a project in OpenStack that appeals to you. If you haven’t selected a project that you like yet, join irc.freenode.org and ask for some guidance in selecting a project to learn to review in either the #openstack-dev or #openstack-101 channels. (The #openstack-dev channel is for all contributors and the #openstack-101 channel is for new contributors.)

Then look for patches to that project on review.openstack.org and read some of the prior reviews to patches. Look particularly for inline comments.

Here are some examples of inline comments to my patches that I found particularly helpful:
This inline comment is a question. It provides a line of code for me to insert, with correct syntax, if I agree with the question as it is asked. This comment identifies the place in the code where the suggested code should be inserted, that is part of the usefulness of inline comments. Finding the right location in the existing code, for the code that I wish to submit, to be inserted is some of the most helpful feedback I can get.

This inline comment provides some feedback in the form of a short part of a stacktrace on the placement of my current code. It then follows up with a suggestion for a better place for my edit to be located. Getting a downvote is helpful, getting a downvote with guidance on how to achieve an upvote is the best kind of downvote.

In this patch I offered some comments on patch set #1 regarding some grammar in the commit message. It may seem like an inconsequential comment, but many contributors to OpenStack do not have English as a first language and a gently worded grammar suggestion is often appreciated. So in this case, my only contribution was some grammar suggestions but it was a contribution good enough for me to review the patch.

Finding nice small patches to review, like this one, can help you gain some confidence. There are still many patches that I just can’t begin to review, this one, is an example. Also this one. This patch is motivated from such a history in the code I haven’t had time to comprehend all that came before so I have no review to offer at this time.

Here is a comment I offered on a minor syntax correction. I used an inline comment to identify the line of code that needed editing, I explained the edit and offered a link to an example of code and then offered a correct code line for the patch composer to copy. These are the kinds of things that help me to improve my patches as I get them ready for approval, so it makes sense to me to offer them to others. Providing patch reviews like this may seem like a lot of work, but the feeling when I get reviews of this nature, with this much supportive detail in them, on my patches makes the work worthwhile.

Find small patches, offer what you can and identify what you are evaluating in the comments. Right now on some patches I can only offer an evaluation of syntax consistency so I state that in my comments.

I hope this post has been helpful to you and I encourage you to start doing some patch reviews if you haven’t already.

Thanks for supporting this GNOME OPW intern,
Anita Kuno.

Comments