Can This Review-Based Git Workflow Be Enforced by Gerrit?

Question

Is there a tool, that makes pull-requests and combined reviews fool-proof and safe in git?

I know that there are a couple of related questions, that have already been asked at github (See also: Using git for Code Reviews, Online Code Review Tool with Git Integration).

People have been suggesting the use of gerrit or gist.

Solutions presented in previous questions have nice interfaces, however they fail horribly when it comes to access control. Our company is too small to force a single person reviewing the code or to have dedicated maintainers. Therefore we are looking for a tool to ensure (or at least encourage) that code is reviewed before it gets pushed to our central repository.

Note: Absolute user access control is not necessary, because we generally trust our employees. However we want to prohibit a push directly to our central repository without restricting the privilege to push to a single person.

Thus the tool (or combination of tools and scripts) should achieve at least these tasks:

• make pull requests visible in a web interface. (gerrit achieves that)
• the central repository is linked to that tool, such that read-only access is possible, but pushing requires at least another person to review and acknowledge the changeset.
• the person responsible for review and acknowledgement can be an arbitrary person from the development team.
• the tool must autonomously detect (and refuse) pull requests that lead to merge conflicts.
• it shouldn't use git functions that are known to alter the SHA1 hash of the commit(s).

My proposal to this solution:

• use gerrit for pull-requests and review handling.
• gerrit should always have a clone of the master.
• for every pull request gerrit checks whether the patches are applicable to master without conflicts (that would be a script hook, and I don't know whether gerrit features them).
• the central repository is owned by a privileged user with shell access (here named gerrit) and is exposed via http(s).
• whenever another person reviews the code, there is an apply button in the web interface that will automatically push the changes to the central repository.

Unfortunately I don't know gerrit and documentation is scarce. Is it possible to implement this workflow in gerrit? Is there perhaps another tool, that fulfils these requirements?

Answer 1

Strictly, you don't need to have gerrit push changes to another repository for hosting (although it has built-in hooks to allow pushing) because Gerrit can act as a repository host.

It's possible to restrict Gerrit to applying only patches which will fast-forward, rejecting those requiring merging. If you've got more than just a few people working on the project, this might slow you down though: the more people commit, the more likely it is that a patch will have to be rebased before being accepted.

Applying patches isn't quite a single-click operation: having reviewed the patch, the reviewer must first select a score (ranging from -2 to +2) with a +2 enabling the "Apply it now" button. If you don't have a CI system verifying the patches, they may also need to indicate that they've verified the source works. If you do have a CI bot, and it's not finished by the time the reviewer looks at the code, they can leave their feedback and anyone (subject to permissions) can trigger the merge when the CI bot is finished.

Your requirement for an arbitrary team member is satisfiable, unless you mean "an arbitrary team member who is not the member who submitted the change". I suspect that's what you actually want, but you might have police this retroactively.

Answer 2

I think Gerrit will meet most/all of your needs. You can integrate a CI tool such as Jenkins which can interact with Gerrit and add additional capability if needed.

One thing to keep in mind - A patch might be able to merge cleanly when the pull request is made, but it could still have merge conflicts later. If developer A makes pull request 1, which can merge cleanly, then developer B makes pull requests 2-9, which can also all merge cleanly, pull request 1 might not merge cleanly anymore if 2-9 are reviewed and submitted first.

Gerrit has capabilities to try and detect this and alert the user when a patch needs to be rebased.