Development/gerrit/PatchReview

    From The Document Foundation Wiki


    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 pin.svg

    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.