Development/gerrit/PatchReview

TDF LibreOffice Document Liberation Project Community Blogs Weblate Nextcloud Redmine Ask LibreOffice Donate

• Wiki Home
• Development
• Design
• QA
• Events
• Documentation
• Website
• Localization
• Accessibility
• Marketing
• Diversity
• Wiki Help

• Overview
• Reporting Bugs
• How to build
• Code Overview
• Git Commands
• Debugging
• EasyHacks
• Release Plan
• FAQ
• Developers
• Extensions
• → Open Issues

After you completed the gerrit setup, and you or others have submitted patches, you can start to review patches on gerrit.

Finding patches waiting for review

./logerrit query status:open

will show you all patches waiting for review. see gerrit documentation for all the fancy things you can do in addition (for scripting etc.).

Getting a patch

./logerrit cherry-pick CHANGEID

with CHANGEID being the Change-Id shown by the output of the gerrit query

Updating patch

If you found and fixed a problem during review. You might update the existing patch in gerrit the following way:

git status; git diff

which makes sure that you have only the single cherry-picked commit and the extra changes on top

git commit -a --amend

which updates the last commit locally

./logerrit submit

which pushes all uncommited changes into gerrit; it updates the existing patch in gerrit if the ID stayed the same in the commit message

Comment and Approve

If you want to add comments or approve the patch, the best way is to do it from the Gerrit web interface. Comment by clicking a line of a changed file and when you are done click the Reply... link to publish your review.

Alternatively, you might just push the patch but it will not add the nice comments about reviewing.

Use of Code-Review and Verified

If you look at your patch on Gerrit you will see two status codes:

• Code-Review
• Verified

The reviewers, our CI system (jenkins) and potentially yourself will use these two fields to qualify the patch.

Code-Review

Can be assigned −2, −1, 0, +1, +2

−2 are to be used by the author, to signal “work in progress”. The −2 prevents the patch from being merged, and only the person who issued the −2 can remove it.

If you work on a larger patch, you are most welcome to upload a patch, mark it as −2, to see if it builds successfully on all platforms

−1 is used by the reviewer, if there are things in the patch that should be changed

0 is used when making comments, that has no effect on whether or not the patch should be merged.

+1 is used by the reviewer, to signal the patch is ok, but the reviewer would like a second opinion

+2 is used by the author to signal no review is needed (this can only be done by core developers, and should be used with care). The person who merges your patch, will use +2, just before merging, since only +2 can be merged. The ability to set +2 depends on the push rights of the reviewer. These rights are conferred individually.

Remark, a patch will NOT be merged as long as there are −1 or −2 unresponded to.

Verified

Can be assigned −1, 0, +1

−1 is used by the CI system if the build fails (note that this is not always a problem in your patch, but can be due to a broken master).

−1 is used by the reviewer, if the expected result cannot be seen.

0 is used when making comments, that has no effect on whether or not the patch should be merged.

+1 is used by the CI system if the build is successful

+1 is used by the reviewer, when the expected result has been verified.

Remark, a patch will NOT be merged unless the CI system shows a successful build.

Note: For more information on those labels, see the gerrit documentation. For gerrit configuration relevant to our projects, see Access Control Lists for all dev-related repositories, for core, and for all other projects.