How to use Gerrit code review effectively

Any code submission to LibreOffice should pass Gerrit code review in order to get merged into the LibreOffice codebase. This is possible using the code review tool Gerrit.

To understand more, refer to these articles:

• TDF Wiki: Development/GetInvolved
• TDF Wiki: Development/gerrit/setup

For getting started with LibreOffice development in general, watch this video.

Here, I assume that you have read those articles, you have created your account, and you have done your first contributions to the LibreOffice – but you want to know more about how to effectively use Gerrit. This is what I am discussing in this article.

How to Find Reviewers

If you are a newcomer, possibly I (Hossein) and Ilmari can help you. You can add us as the reviewers, and we will review your submissions. Just search for our names in front of the “Reviewers” part in your submission, and you can easily add me or Ilmari as the reviewer.

If you want to work on more complicated tasks, you can also refer to this page to find experts in different areas.

• TDF Wiki – FindTheExpert

We have experts in these areas:

Access2Base, Automated tests, BASIC, Bugzilla, build system (gbuild), Calc, Chart, Database, Debian, adding dictionaries, documentation, Draw, General UI problems, git⁠ and Writer RTF filters, gerrit⁠, GUI design questions, headless (–without-x build switch), i18nlangtag, i18npool, Impress presentation, KDE, Libcdr and Corel Draw import, libvisio and Visio import, localization fixes / l10n / non-English bug description, localization fixes / l10n tools, Mac, ODF filter, Pebble, PostgreSQL integration, QA, ScriptForge, Translation, Ubuntu, UI, UNO, sal, configmgr, VCL, Weblate, Wiki, Wiki Extension, Wiki Config, Editing, Windows, Windows Installer, Writer, writerfilter, X11-specific

Quite a lot of topics!

But, as you can read in that Wiki page, please “Please do not email these developers personally with your personal pet bug”. Just add them as the CC or the reviewer of a relevant submission.

Handling Problems with the Jenkins builds

Sometimes it happens that Jenkins can not build your submission. This can happen for several reasons. It can be because of a problem in your code, or a problem with the build system itself.

If the problem is from your side, then fix the problem and re-submit your code as a new patch set, and Jenkins will try to build it again.

But, if the problem is with Jenkins, you can either ask on #tdf-infra and someone with appropriate access will resume your build. Another solution would be doing a re-base and then Jenkins will build your submission again. For doing this, you do not need any special permissions. This will rebuild for all the platforms, so if one or two platform builds has failed, it is better to asking on #tdf-infra. Also, if all the platform builds have failed, you may need to check your code first!

Please be patient, and keep in mind that CI is a limited resource, so you should use it carefully. Please note that you should build (using make) and test (using make check) on your computer locally before submitting the code to the Gerrit.

Working on Multiple Gerrit Submissions

Sometimes you want to work simultaneously on multiple submissions. Then, you have to create a branch for each of these submissions to be able to work on them.

A tool named git review can help you manage your submissions easily. It is a review tool, but you can use it both for submissions from others (for reviewing), and also your own submissions.

To download a submission with the id of 1234, you can easily do:

git review -d 1234

The id (1234 here) is the number you see in the end of the submission URL, for example: https://gerrit.libreoffice.org/c/core/+/1234. Then, your patch is downloaded, and a new branch is created for that submissions.

To download a specific patch set, add it after the id with a comma like this:

git review -d 1234,7

This will download patch set 7.

Fixing Merge Conflicts

If your patch is more than a week old, please do a re-base. This can be done in the Gerrit web UI, or if you want to do a re-base locally, do this:

./g pull -r

Sometimes, you can see “merge conflict” in the Gerrit UI. Also, you may face a merge conflict locally doing a re-base. In this case, you have to download your changes (for example using git review), do a re-base, and then try to resolve the conflicts. Then, you have to use git add to stage the changed source files.

When you have finished fixing your submission, you can do:

git commit --amend
./logerrit submit master

If you want to go back to the master branch, just use:

git checkout master

Handling Requested Changes From the Reviewers

In the review process, you should do the changes requested from the reviewers. Sometimes, you may disagree with the reviewers, but then you should provide good reasons for not doing the requested changes in order to convince the reviewers.

Anyway, you should know that your code must conform to certain requirements in order to be eligible for merging. So, try to convince the reviewers that your code deserves to get merged into the code.

For the code conventions and coding style, it is suggested that you refer to these links:

• C++ coding standards (old but still usable)
• Writer code conventions (also old) specifically the naming conventions

For newer files, you may have to use clang-format, to re-format the code, and convince the CI that your code is fine!

There are many rules in the above links. Among them, this list on variable name prefixes is very useful. The list is extracted and suggested by Mike and Heiko:

• (Smart) pointer/reference to an UNO interface -> xName
• (Smart) pointer to anything else -> pName
• Reference to anything else -> rName
• Integer value -> nName
• Floating-point number -> fName
• Boolean -> bName, or isName, or canName
• String -> optionally sName (for char * -> pName; for OUString -> aName)
• Another object -> aName

Other conventions are gathered as extensive set of rules. If you had any doubts, you can refer to the appropriate section that discuss design, implementation, testing and other aspects.

On the other hand, you should know that the easiest part of the change is the coding style! There are many things related to good design and implementation that you should take care to be able to pass the code review and get a +2.

Search Among the Gerrit Submissions

By clicking on the name of each person in the Gerrit, you can see their contributions. But, this is not the whole story! Gerrit has a powerful query language that you can use inside the search box. For example:

owner:self: My own contributions

reviewer:self: Submissions that I am reviewing

owner:self status:merged: My merged contributions

owner:self status:open: My open contributions

You can use boolean operators between the queries:

AND -(negation) OR

If you do not specify anything, then AND is used.

You can do a lot of complicated searches among the submissions using its manual.

Final Remarks on Gerrit Code Review

Gerrit is a powerful tool, and you can use many features of it to work more effectively. But, keep in mind that your focus should be on fixing the possible problems of your code, and improving your code quality.

Don’t worry! Reviewers usually help you on the road to fix the problems. Although in the end, it is your responsibility to address the concerns and do the recommendations – and eventually fix your code.

To get more help, you can always refer to its manuals.

I hope that you will get the best out of this review tool!