CODEWITHSUNDEEP
gitworkflowgerrit
Alexander Oh
Alexander Oh

Reputation: 25641

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

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:

My proposal to this solution:

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?

Upvotes: 3

Views: 1311

Answers (3)

odinho - Velmont
odinho - Velmont

Reputation: 21506

I sincerely recommend Critic, a Git review system developed and used at Opera Software. W3C also use it for reviewing tests.

make pull requests visible in a web interface. (gerrit achieves that)

Critic does that. And it's totally integrated with Git. You just set critic as a Git-remote, and push to r/your-branch and it will create a review, email all the matched reviewers for those files.

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.

Critic indeed has its own repository. It comes with a few hooks to do different things, but for your use case you'd probably write some yourself.

the person responsible for review and acknowledgement can be an arbitrary person from the development team.

This it can. Or actually, you (as reviewer) put up a review filter for stuff you want to review. There's also 'watch' filters. In small repos I just do a filter on / (everything). On bigger projects I have e.g. /desktop/linux/ and e.g. one with .*.py.

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).

It's much better, it can do both. The way we use it is with rebasing. You can do it yourself, and it will figure out what you changed. If you change stuff AND do a rebase, it gets rather angry with you, but will show all the changes that happened in a 3-way review.

We have an extension that most users have installed, FiddleAndTweak, which allows you to do simple fixups directly in the Review interface, as well as Interactive Rebase before you push.

We have our own commitqueue which Critic is connected to using a simple extension. That allows you to queue your change with one click after the review is accepted. It won't allow you to push unclean branches. It's the commitqueue itself that does that actual compilation and testrunning before merging; but you can allow it to either merge or rebase your changes. Sadly our commitqueue is very specific and not open sourced like Critic has been. Although the review system is definitely the harder part.

So with critic, you'd need to write some small scripts outside in order to do the "push to repo". But for some simpler projects it's very easy to write a small extension that just merges the accepted code to master when the 'integrate' button is pushed.

Upvotes: 0

Andrew Aylett
Andrew Aylett

Reputation: 40720

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.

Upvotes: 1

Brad
Brad

Reputation: 5532

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.

Upvotes: 1

Related Questions