Live data from Hacker News

Git-appraise – Distributed Code Review for Git

github.com

91–100 of 173 posts

Re: Git-appraise – Distributed Code Review for Git

#93
Yay git notes! I've been meaning to dig into building something exactly like this with them, because I hate how reviews and comments are basically a lock-in mechanism.

There's a lot of room for building interesting stuff with notes, because they let you attach data to commits (and therefore files, lines, etc) without changing the SHA. E.g. got a question about a change months after it's committed? You can add a note to it, and now everyone can benefit from the Q&A. IMO it has just been missing a killer feature implementation or two, to spread knowledge that it exists and get more tools to expose them. They're invisible on GitHub's UI, for instance.

Re: Git-appraise – Distributed Code Review for Git

#95
post #83

Earlier quoted context omitted.

The same way you deal with any other conflicts, you won't be able to push without pulling first.

Which I can imagine could be annoying in some situations :-). But no workflow is perfect. I think I like the idea that in an e-mail, I can really comment on some patch, and then the author of the patch can decide what to do with it. Maybe the author modified the patch in the meantime, but they will know if my comment is still valid or not. Without me having to handle the conflicts. I'm thinking mostly about leads who…

Deleting three lines to keep both comments doesn’t seem too bad. You might even just delete your comment entirely if it’s for the same thing.

Re: Git-appraise – Distributed Code Review for Git

#96
post #61
post #44

I have this "one weird trick" that I think is great, but somehow no-one else wants to use: At one point I worked with a fellow dev where we did PR reviews just as commits on top of the branch. So you'd check out their feature branch, run the equivalent of 'compare working tree with ' for your editor and then look through the changes. Because you're comparing the working tree you can leave comments... by typing source…

Have you ever tried the git e-mail workflow? I think you may like it, I'd be curious to have your opinion about how it compares to your workflow above :-). The idea is that you just send your patch by e-mail, and the reviewer just applies it with `git am `. So you don't even have to sync on a branch. Then the reviewer can comment directly on the patch. You answer with new versions of the patch, and get new reviews. I…

I have been giving this one a lot of thought. I jumped to sourcehut for my personal projects and was amazed how viscerally pleasing a [PATCH] email thread could be. I am convinced there is a big psychological difference between discussions via Pull Requests and discussions via email/mailing lists. I can’t pin point it but it is almost palpable.

A question I have with the process is: can email based workflow still work with for a continuous integration/delivery pipeline?

Re: Git-appraise – Distributed Code Review for Git

#97
post #44

I have this "one weird trick" that I think is great, but somehow no-one else wants to use: At one point I worked with a fellow dev where we did PR reviews just as commits on top of the branch. So you'd check out their feature branch, run the equivalent of 'compare working tree with ' for your editor and then look through the changes. Because you're comparing the working tree you can leave comments... by typing source…

You're not crazy and not alone. At Hydraulic we do the exact same thing. It has other advantages you didn't mention: • It yields much more collaborative code reviews, because for small tweaks it's quicker to just fix the thing you want changed than ask someone else to do it. After all, you've already got the code checked out and in your IDE. This is especially true when working with easily refactorable languages like…

"it's quicker to just fix the thing you want changed than ask someone else to do it"

Sure, but I feel like there's a big disadvantage here in that the person who did the bad thing isn't actively involved in fixing it. They lose out on acquiring that muscle memory. If this is a Sr. reviewing another Sr.'s PR and it's just a few minor oversights, fine. But if this is a Sr. reviewing a Jr.'s PR and they fix like 10 things in one commit, that seems harmful to the inexperienced dev's development. They don't even get a chance to ask why or provide clarification.

And if it comes off like a bunch of nitpicking, that's a person who likely really needs a little handholding. The whole "give a man a fish vs teach" proverb.

Re: Git-appraise – Distributed Code Review for Git

#98
post #44

I have this "one weird trick" that I think is great, but somehow no-one else wants to use: At one point I worked with a fellow dev where we did PR reviews just as commits on top of the branch. So you'd check out their feature branch, run the equivalent of 'compare working tree with ' for your editor and then look through the changes. Because you're comparing the working tree you can leave comments... by typing source…

I'd say the biggest drawback of that approach is churn; loads of extra commits and code changes for code review comments. Of course, that becomes a non-issue if you use a squash merge approach, instead of retain all commits (which in this approach have a lot more of a "WIP" feel to it). Do code review comments have value after merging? I would also posit they're easier to find if they're in an external location (like…

I assumed the code review commits would be squashed before the merge. But you raise a valid point about the code review process being lost. One way to get around that is to keep the feature branches (or maybe just tags) forever and do all the squashing on the integration branch. This does require some kind of convention that records on which branch/tag the code review for a particular feature can be found.

The advantage of this kind of approach is it doesn't require any special tooling, but could certainly be made easier with special tooling.

Re: Git-appraise – Distributed Code Review for Git

#99
post #70

Reading the comments, I am quite surprised to see no mention of the git e-mail workflow. Git was designed for it, but somehow it seems like it has been completely forgotten. Drew Devault has nice posts about it: https://drewdevault.com/2018/07/02/Email-driven-git.html https://drewdevault.com/2022/07/25/Code-review-with-aerc.htm...

https://news.ycombinator.com/item?id=37085273 and https://news.ycombinator.com/item?id=37086360

Re: Git-appraise – Distributed Code Review for Git

#100
post #24

Storing code review (and ci results, and analyzer runs) in git is a wonderful idea. All these workflows are a derivation of the source in the repository and keeping them close together has a great aesthetic. It also enables the 'single pane of glass' to observe the a pull request from.

> All these workflows are a derivation of the source in the repository and keeping them close together has a great aesthetic.

I agree. Version control is a great enabler, so using it to track "sources" other than just code can be useful. A couple of tools I like to use:

- Artemis, for tracking issues http://www.chriswarbo.net/blog/2017-06-14-artemis.html

- ASV, for tracking benchmark results https://github.com/airspeed-velocity/asv (I use this for non-Python projects via my asv-nix plugin http://www.chriswarbo.net/projects/nixos/asv_benchmarking.ht... )

Post reply on HN