Review code

TeamForge provides a unified code review experience as it supports both Pull Request and Gerrit single-commit reviews.

Git repositories are hosted and served in TeamForge via Gerrit. Gerrit is most widely known for providing powerful code review features. While Gerrit includes a powerful code review feature, the way it works and the workflow is different from the Pull Request style that was introduced by GitHub. A Google search of “Gerrit Pull Request” will yield a bounty of passionate viewpoints on these differences. Hashing through the pros and cons would just add another result to that search. Instead, just know that with TeamForge you are free to use either style of code review methodology, even on the same Git repository.

While TeamForge supports both pull request and single-commit Gerrit reviews, this topic focuses more on the Pull Request type reviews.

Pull request configuration

In order to use pull requests in your Git repository there is some configuration that must be done first. This is to set up the proper permissions in your repository so that your policies are being followed.

Open the repository in the code browser, click the Settings tab then click Policies tab. This is where you configure the pull request-based code review policy. For more information, see Configure Pull Request for repositories step-by-step.

Repository Category and Protected Branches: A new category named “Pull requests” has been added. What this category does is set up the repository permissions so that users can create and push to feature branches but require pull requests to certain “protected branches”. Once you change the repository category to "Pull request", the Protected Branches field shows up. This will be the list of branches that you will be merging your pull requests into. Typically this would be the “master” branch but you may also have various “release” branches that you would like to protect. Users will not be able to directly push changes to these protected branches. Instead, the user will create a feature branch with their changes in it, and then create a pull request when they are ready for their changes to be reviewed and merged to the protected branch.

Review Rules: These review rules govern the requirements for a given change to be eligible to be merged. There are four new policies available:

No Approval Required: This is similar to the policy on sites like Github. This basically means anyone with the proper TeamForge permissions can accept and merge any pull request. This means you probably favor “social” policies and trust that your reviewers will do the right thing. Pull requests become a tool to aid with code review and it is still possible for users to use the voting tools in the review to communicate their feedback but the votes on a review do not prevent the review from being merged.
Code Review Required: With this policy, the voting tools begin to matter. The change cannot be merged until it has a net positive vote total, not counting the owner of the review. In other words, if two users give a thumbs up and one a thumbs down then it will be eligible to be merged -- assuming the owner of the review is not one of the two thumbs up. The owner can vote, but their votes do not count towards the total.
CI Required: The assumption here is that the relevant votes are being cast by a “bot” or “process” such as a Jenkins CI job. There is no UI in the pull request provided to cast these votes, it will be done via API or by using the Gerrit UI. The pull request shows a check mark when a positive Verified vote has been cast and an X when a negative vote has been cast. With this policy the change cannot be merged unless there is at least one positive verified vote and no negative votes. Users can still provide thumbs up and down votes but they do not control whether or not the change is eligible to be merged. Of course the person that decides to merge the change can still factor in the code review votes and comments.
Code Review and CI Required: This is obviously just a combination of the two previous policies. So a positive Verified vote is necessary, with no negative verify votes, and a total positive Code Review vote is required.
Default: The final policy is to just use the Gerrit code review default policy. This requires a +2 code review vote and a +1 verified vote and there cannot be a -2 code review vote as that acts as a veto. Users that are not familiar with Gerrit tend to find these voting rules confusing. For example, two +1 votes does not equal a +2 vote and your permissions determine what votes you are able to cast. We do not recommend you use this policy if you are using pull requests, but it is an available option and might be desired if you are already using Gerrit reviews and do not want to change the voting rules.

Pull request workflow walk-through

The primary difference between the pull request workflow and the normal Gerrit change-based model is the use of branches. In the pull request model the assumption is that work will begin on a feature branch and you create a pull request when you are ready to start receiving feedback on the branch. This could mean the work is ready to be merged, but it could also mean that you just want to get feedback from the CI system or initial feedback from code review. Once all feedback and review is complete and the pull request is eligible to be merged, then the request can be merged and the feature branch deleted.

Create feature branch (locally)

If working in a small team you might want to create the feature branch on the server or create one locally and push to the server right away. For now, we will assume that there is just a single developer. Typically, it is best to just begin the process by creating a feature branch locally. It is a good idea to fetch all changes from the server before beginning this process:

$ git checkout master && git pull origin master
$ git checkout -b feature_branch

Give your feature branch a meaningful name.

Commit to feature branch and push to server

The next steps are of course to just do your work and commit changes locally. Before you have pushed the changes to the server it is OK to do things like squashing your commits or rebase your branch on master, but once you have pushed your branch to the server you should no longer do this. The first time you push to the server you will need to set the upstream branch to the name you want to give your feature branch on the server.

$ git push --set-upstream origin feature_branch

Create pull request

When you are ready to merge the change, or at least to start getting feedback, you should create a pull request, add reviewers and have reviewers share feedback on your changes. See Create Pull Request step-by-step for more information.

Pull requests are implemented as merge commits between your feature branch and the target branch. The pull request subject and description will combine to form the commit message for the merge commit. You can also provide a Markdown summary of the change that will be captured as the first comment on the pull request. If no summary is provided, then the commit message will serve as the first comment. If you have automated CI configured, then it will typically run as soon as the pull request is created and whenever it is updated. So you could also create a pull request early in your process so that you can benefit from the feedback of your CI system. If you are posting a pull request that is not ready to be merged it is a good convention to follow to cast a Thumbs Down vote in the pull request to signify this to potential reviewers.

Reviewers: Reviewers are anyone that you potentially want to provide feedback on the change. Adding a user does not require the user to review the change, it just notifies the user of its existence and see it in lists and filters of reviews they are assigned to. Likewise, users that have not been added to the review are still free to cast votes on the review.
Voting: Tools are provided to cast votes on the review. If you are using one of the four code review policies provided with TeamForge you will see simple Thumbs Up/Down buttons to cast your vote.

Commit and Push More Changes

As you continue to work on your feature branch you can just commit changes to your local branch and push them to the server. If you are working in a team on the same branch, then you will need to fetch and rebase changes made by other team members to the feature branch.

$ git add/commit etc.
$ git push

Update Feature Branch and Pull Request

If new changes are pushed to the feature branch, the pull request must be updated to include the new changes. This involves updating the merge commit to recognize the new HEAD of the feature branch. To update the pull request you simply need to open it in the web UI. It will then update itself as needed.

Note: When a pull request is updated, all existing votes will be reset and need to be cast again based on the new review.

Resolve Conflicts

When working in a feature branch, it is not uncommon for conflicts to arise between your feature branch and the target. When this happens, you will not be able to merge the pull request and you will see a warning of the conflict in the web UI. To resolve the conflicts you must fetch and merge the changes from the target branch into your feature branch and resolve and commit the conflict resolution. Then push the result back to your feature branch on the server and update the pull request.

$ git fetch $ git merge origin/master
$ git add/commit etc. $ git push

Of course you can also rebase and force push to update your feature branch as long as you understand the ramifications of this when collaborating with a team that is sharing the same branch.

Merge Pull Request

Once a pull request is eligible to be merged, meaning there are no merge conflicts with the target and all voting requirements have been satisfied, the Merge button will be enabled in the web UI. Anyone who can see this button has the permissions to merge the pull request and just needs to click the button to merge it. See Merge pull requests step-by-step for more information. This ends the life cycle for this pull request and if the user has the necessary permissions they will also be given the ability to delete the feature branch from the server.

It is possible to continue to use the feature branch and create new pull requests to merge subsequent changes, but this is not recommended. It is generally a good idea to delete feature branches once they have been merged.