Martin Stoyanov recently asked an excellent question about the proper way to push revisions to a changeset in Gerrit. This is a very common question that folks new to Gerrit or Git have, and I think it deserves its own post.

When you do the first push of a local working branch to Gerrit, the act of pushing your code creates a Gerrit changeset. The changeset can be reviewed, and in the process of doing that review, it’s common for reviewers to request that the submitter make some changes to the code. Sometimes these changes are stylistic or cosmetic. Other times, the requested modifications can be extensive.

How you handle making the requested modifications and submitting those changes back to Gerrit depends on a few things:

1. Are the changes requested mostly stylistic or cosmetic?
2. Are the changes requested going to provide additional functionality that is dependent on the existing changeset?
3. Are the changes requested going to provide additional functionality that is independent of the existing changeset?

Depending on the answers to the above questions, you should either amend the existing changeset commit, push a new commit to Gerrit from the same local branch, or push a new commit from a new local branch. Here’s some quick guidelines to help you decide:

The Changes Requested Are Cosmetic or Style-related

When a reviewer is providing some stylistic advice or offering suggestions for cosmetic changes or cleanups, you should amend the original commit. Do so like this:

# After making the requested changes...
git commit -a --amend
# Modify the commit message, if necessary, and save your editor
git review

While this looks fairly simple (and it is…), many folks make a fatal mistake when they modify the commit message and add sections that describe the “sausage-making” involved in the cleanups. DO NOT do this. It’s not necessary. Avoid adding any lines to the commit message that look like this:

• “Cleaned up whitespace”
• “DRY’d up some stuff based on review comments”
• “Fixed typos found during reviews”

If all you did was correct typos and whitespace, simply leave the commit message as it was originally. After the call to git review, you will see a new patchset appear in the original code review. This is expected. The changeset is still viewed by Gerrit and reviewers as a single changeset, and reviewers may even select “Patchset 1” from the “Old Version History” dropdown instead of “Base” in order to see only the changes made in this last amended commit.

The Changes Requested Are Extensive and Depend on Original Commit

If a reviewer has asked for modifications to your original code, and the requested modifications are fairly extensive and depend on the code in your original commit, you have four choices:

• Amend the original commit to include all new changes
• Amend the original commit for some things, push those changes to the original commit, make additional changes in the same local branch, git commit those additional changes and git review
• Make additional changes, git commit those additional changes and git review
• Lobby for your original commit to be accepted as-is, then when your change is accepted and merged into master, then create a new branch from master and push the additional changes in a new changeset

Whenever you do not amend a commit and issue a call to git review, you will be created a dependent changeset. Gerrit will assign a new Change-Id to the patchset, but understands that the commit logically follows your original changeset’s code. If you go to the code review screen of your newly-created changeset, you will see your original changeset referenced in the “Dependencies” section. Below, you can see a screenshot of a changeset that is part of a “dependency chain”. Another patchset is dependent on this patchset and this patchset is dependent on another patchset.

It’s best to avoid long chains of dependent patchsets. The reason is because if a reviewer requests changes for one of the changesets at the “bottom” of the dependency chain, the entire chain of changesets (even changesets that are approved like the one shown above) are going to be held up from going through the gate tests.

The Changes Requested Are Extensive but are Independent of the Original Commit

If a reviewer has requested extensive changes, but points out that the changes they want made are actually independent of the changes in your original commit, the reviewers will generally ask the original committer to wait until this changeset is merged and create a new branch for the additional work. Normally, depending on the extent of the requested changes, reviewers will insist that the submitter create a new bug or blueprint on Launchpad to keep track of the additional work they feel is needed.

Conclusion

It’s up to the discretion of core reviewers and the original submitter to work out which of the above solutions works best for the particular changeset. Each changeset introduces code and functionality that must be treated differently, and changesets from one submitter may be dealt with differently than others. However, to keep things simple for yourself and upstream reviewers, it’s best to follow this simple advice:

1. Prefer to amend the original commit. In most cases, this is the appropriate solution to push revisions to Gerrit.
2. Don’t include sausage-making comments in the commit message.
3. Prefer free-standing changesets to long chains of dependent patches.
4. Ask reviewers what their preferences are.

Follow those guidelines and you’ll keep yourself out of the weeds. For more detailed information, including strategies for handling updates to your code that is dependent on another branch of code that gets updated, see the excellent OpenStack GerritWorkflow documentation.