3 ms·

Graphite is neat. If you want something lighter-weight, spr[1] is also worth taking a look at as an entirely client-side "solution" to PR stacking. Unfortunate

by lima 3y ago

Graphite is neat. If you want something lighter-weight, spr[1] is also worth taking a look at as an entirely client-side "solution" to PR stacking. Unfortunately, it is very hard to build good code review tooling on top of GitHub due to the severe impedance mismatch between the two models as well as GitHub API limitations and rate limits. That mismatch cannot be fully resolved (branches vs. patches) and results in constant friction, and you end up trusting a third party with full control over your repositories and approval workflows. Graphite is the best attempt I've seen so far, but it still doesn't come anywhere close to Gerrit[2], which simply uses plain Git commits. Every commit becomes a review, and stacking is accomplished by simply pushing multiple commits. No custom tooling required - just `git push`. It has very, very good code review UX and allows meaningful and in-depth back-and-forth during a code review, without losing context or having to re-review the entire diff. Once you're past the initial learning curve, it is blissful. Gerrit is open source and it's what Google uses for many of their public projects such as Chromium and Android, and it is quite easy to self-host. It is entirely built on JGit and even stores code reviews as Git commits alongside the repositories. If you want to give it a try - there's a well-maintained public instance, Gerrithub[3], operated by Gerritforge. Many open source projects use it. [1]: https://github.com/ejoffe/spr https://github.com/ejoffe/spr [2]: https://www.gerritcodereview.com/ https://www.gerritcodereview.com/ [3]: https://gerrithub.io https://gerrithub.io

Zacharias030 3y ago
Shoutout to ezyang‘s ghstack as well! ghstack also adopts the one-PR-per-commit paradigm, which means you have to get comfortable with a lot of amending and interactive rebases and moving fixup commits around your stack. Amendments are nicely mapped to new commits in the PR though. Graphite really has nice support for that sort of stuff and opens PRs directly against master. I don’t really like that it needs an external service beyond just github and git, but oh well. The only thing I had problems with is that merging from github is very confusing and I don’t want to be forced to adopt their webinterface. I‘d prefer to merge/land my stacks from the CLI.
Borealid 3y ago
You mentioned a lot of Gerrit features but you didn't mention two: - it stores review information as "git notes", so you can see in the repository itself with `git show` who reviewed which commit and what score it was given, even though it doesn't alter the commit hashes by editing the commit message - you can have arbitrarilty many different types of review, for example a QA test score that's separate from a code review score also separate from unit test scores. Much better than the all-builds-green other options offer Gerrit is the best code review tool I've ever used and I don't get why people copy Github instead of Gerrit.
- aseipp 3y ago
  It also has a significant amount of detail in subtle ways, some that can't really be appreciated until you try it. My go-to example of this is the "Attention Set," which came directly from Google's Critique. In short, it just tells you "Who should look at this change next," i.e. if you publish a change, it's the reviewers turn. They leave comments you need to fix, so now it's your turn. And so on and so forth. That's a feature that, in retrospect, seems instantly obvious and "duh", but it's something that actually is not obvious to the designer unless you have spent a lot of time in the general problem space. Gerrit is a really great, productive tool.
marco_patino 3y ago
[dead]
bryanlarsen 3y ago
https://stacking.dev/ https://stacking.dev/ lists 5 tools to help with stacking, including spr and Graphite. AFAICT Git Town is the only one that works with GitLab.
aseipp 3y ago
The key part of the data model mismatch, one that could be solved, is that Git does not natively give you a way to refer to a commit by a "logical ID", one that remains stable under rebases and content rewrites, i.e. a unique identifier that allows you to identify a change anywhere in the commit graph. The commit hash is more like a "physical ID", because it is derived purely from the contents of the change. Version control systems like Jujutsu natively support the concept of a logical ID, which is called a "Change Id", and is exactly the same kind of ID used by Gerrit in its `Change-Id: ` footer. There has been some discussion before on the Git development list about adding Change Ids directly to Git, which would solve a tremendous amount of these problems. You'd never need to have commit-msg hooks and forges could natively support Change IDs easily. I don't think this idea was ever shot down. As usual it's mostly a matter of "someone's gotta do it" and go hack on the big Git codebase. (I don't have a link right now, sorry.) Gerrit is probably one of the two best code review tool/development flow I've ever used; though, I liked many of Phabricator's design and UX decisions a lot more, to be frank. (Disclosure: I work on Jujutsu. Yadda yadda.)
- lima 3y ago
  Phabricator is what we used before we migrated to Gerrit. The review UI was very nice, the issue tracker is still best-in-class, but Arcanist was a menace. Much prefer a simple "git push" over that client-side PHP contraption :-) Are there plans for Gerrit interop with Jujutsu?

by lima 3y ago

Zacharias030 3y ago

Shoutout to ezyang‘s ghstack as well! ghstack also adopts the one-PR-per-commit paradigm, which means you have to get comfortable with a lot of amending and interactive rebases and moving fixup commits around your stack. Amendments are nicely mapped to new commits in the PR though. Graphite really has nice support for that sort of stuff and opens PRs directly against master. I don’t really like that it needs an external service beyond just github and git, but oh well. The only thing I had problems with is that merging from github is very confusing and I don’t want to be forced to adopt their webinterface. I‘d prefer to merge/land my stacks from the CLI.

Borealid 3y ago

You mentioned a lot of Gerrit features but you didn't mention two: - it stores review information as "git notes", so you can see in the repository itself with `git show` who reviewed which commit and what score it was given, even though it doesn't alter the commit hashes by editing the commit message - you can have arbitrarilty many different types of review, for example a QA test score that's separate from a code review score also separate from unit test scores. Much better than the all-builds-green other options offer Gerrit is the best code review tool I've ever used and I don't get why people copy Github instead of Gerrit.

aseipp 3y ago
It also has a significant amount of detail in subtle ways, some that can't really be appreciated until you try it. My go-to example of this is the "Attention Set," which came directly from Google's Critique. In short, it just tells you "Who should look at this change next," i.e. if you publish a change, it's the reviewers turn. They leave comments you need to fix, so now it's your turn. And so on and so forth. That's a feature that, in retrospect, seems instantly obvious and "duh", but it's something that actually is not obvious to the designer unless you have spent a lot of time in the general problem space. Gerrit is a really great, productive tool.

marco_patino 3y ago

[dead]

bryanlarsen 3y ago

https://stacking.dev/ https://stacking.dev/ lists 5 tools to help with stacking, including spr and Graphite. AFAICT Git Town is the only one that works with GitLab.

aseipp 3y ago

The key part of the data model mismatch, one that could be solved, is that Git does not natively give you a way to refer to a commit by a "logical ID", one that remains stable under rebases and content rewrites, i.e. a unique identifier that allows you to identify a change anywhere in the commit graph. The commit hash is more like a "physical ID", because it is derived purely from the contents of the change. Version control systems like Jujutsu natively support the concept of a logical ID, which is called a "Change Id", and is exactly the same kind of ID used by Gerrit in its `Change-Id: ` footer. There has been some discussion before on the Git development list about adding Change Ids directly to Git, which would solve a tremendous amount of these problems. You'd never need to have commit-msg hooks and forges could natively support Change IDs easily. I don't think this idea was ever shot down. As usual it's mostly a matter of "someone's gotta do it" and go hack on the big Git codebase. (I don't have a link right now, sorry.) Gerrit is probably one of the two best code review tool/development flow I've ever used; though, I liked many of Phabricator's design and UX decisions a lot more, to be frank. (Disclosure: I work on Jujutsu. Yadda yadda.)

lima 3y ago
Phabricator is what we used before we migrated to Gerrit. The review UI was very nice, the issue tracker is still best-in-class, but Arcanist was a menace. Much prefer a simple "git push" over that client-side PHP contraption :-) Are there plans for Gerrit interop with Jujutsu?