Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Some of us like "interdiff" code review

gist.github.com

Post reply on HN
Print view View original

111–120 of 224 posts

Re: Some of us like "interdiff" code review

#111

Post by aseipp »

dogleash wrote: post #110
aseipp wrote: post #108

Earlier quoted context omitted.

I mean, the answer is simple. Even if email-based flows use range-diff, which is the correct conceptual model, all the actual details of using email are, I would estimate, about 1,000x shittier in 2024 than using GitHub in 2008 when I signed up for the beta as user #3000-something. Email flows fucking suck ass. Yes I have used them. No, I won't budge on this, and no, I'm not going to go proselytize on LKML or Sourceh…

Dude, I'm not making a defense of mailing list workflows here. I'm just pondering the nature of the world where despite all the yapping about git I've seen floating around on the internet for as long as I've been lurking social media, the yappers are just recently keying in on something.

If you're asking "Why did this take 15 years for people to understand" and my reply is "Because it was under 1000 layers of other bullshit", then that's the answer to your pontification. It has nothing to do with whether you think email is good or not. You pondered, I answered. That simple.

Re: Some of us like "interdiff" code review

#112

Post by amstan »

aseipp wrote: post #31
zeotroph wrote: post #29

Earlier quoted context omitted.

From my interaction with the free part of GitHub, "diff soup" describes it very well. Does the paid version do anything better? What about GitLab, can this get near Gerrit? And then there are the external services which try to make GitHub less painful (and quite pricey, especially compared to a selfhosted Gerrit), by providing stacked diff support, did you look at these?

No, paying for GH doesn't make the code review experience any better. It's identical across public/cloud/enterprise GH. I do not know if GitLab does anything different; I've never used it in anger. I'd bet $10 the answer is "no, it's basically just the same as GitHub", though. If you want a service that adds stacking on top of GitHub, my conclusion after some research is that https://graphite.dev/ is the best option.…

Does Graphite have a gerrit instance or something? I'm prepared to say "shut up and take my money" compared to the other 20k/year offer.

Re: Some of us like "interdiff" code review

#113

Post by planetpluta »

aseipp wrote: post #102
planetpluta wrote: post #99

I’m confused by how pushing to new branches would work on GitHub (or is the point that it doesn’t…)? Are you able to change the branch of a PR from `v1` to `v2` without making a new PR?

Yes, the point is that it basically doesn't support that. Well, OK. You can push two branches, v1 and v2, each with the commits. Then to do pairwise diffs, you type in the commit object hashes directly into the URL bar to diff the two objects in the repository using the 'blobs' API but like... I don't think that qualifies so much as "supporting" it as much as an absurd hack, right? > Are you able to change the branch…

Okay that makes sense. Agreed, wouldn’t quite consider that “supporting” as far as I’m concerned.

Re: Some of us like "interdiff" code review

#116

Post by hoten »

Some of the problem stated in the post is a little forced, namely the issues with bisect and blame. That's only an issue if the review doesn't end with a squash.

Also as a user of gerrit via the Chromium project, I'm not aware of a way to structure the patch sets uploaded as individual changes without infinite foresight. It's always appended into the last. Whereas with github, at least you can rebase. I fully admit I could be missing a gerrit feature.

That said... I would really love an interdiff feature for the GitHub rebase-to-fold-in-feedback-to-meaningful-commits workflow.

Re: Some of us like "interdiff" code review

#117

Post by jrochkind1 »

I have found myself arranging PR's on github in sequence, for the exact sort of use case OP talks about. (Each PR might have more than one commit, so it's not exactly the same practice). And been frustrated that Github's interface does not make it very easy to make the sequence apparent or have good DX for the reviewers.

One could definitely imagine a UX/DX that would. (I do apprecaite github's UI generlaly).

Re: Some of us like "interdiff" code review

#118

Post by jrochkind1 »

mizzao wrote: post #97
Vinnl wrote: post #83

I'm using mostly this workflow with GitHub, with the main disadvantages being that it's more work on my side, and not obvious to my collaborators. But it does carry the same advantages of allowing reviewers to view diffs with just their feedback incorporated, without breaking `git blame` and `git bisect`. When I incorporate a reviewer's feedback, I'll commit that with `git commit --fixup `. I'll then push that up and…

I was going to say... interactive rebase addresses a lot of the "diff soup" comments that the writer complains about. It's really only done by disciplined engineering teams though (who bother to learn some more advanced features of git)

I realized reading the first part of this article that I often want to set up a sequence of PR's, for very similar reasons as in the begininng of OP. Say, a prefatory refactor, then the main work, then some data cleanup.

When I do this, the problem with rebase is that it kind of breaks the additional "next in sequence" PRs "on top", or at least requires (confusing to me) cleanup in all of them when I rebase the base.

Re: Some of us like "interdiff" code review

#119

Post by WorldMaker »

zeotroph wrote: post #29
aseipp wrote: post #15

I am the person who wrote this. AMA EDIT: Also, I'm not sure if this is against the rules, but I also need a new job as of recently. I like working on dev tools and other hard problems. If you liked reading this, want me to make your dev team more productive, or just want to experience and enjoy my excellent (and occasionally eclectic ) taste, the email is in my profile.

From my interaction with the free part of GitHub, "diff soup" describes it very well. Does the paid version do anything better? What about GitLab, can this get near Gerrit? And then there are the external services which try to make GitHub less painful (and quite pricey, especially compared to a selfhosted Gerrit), by providing stacked diff support, did you look at these?

GitHub does have some stacked diff/"merge train" tools on the deeply paid side (the "Call Us" sorts of Pricing tiers) that I've only seen screenshots and demos of.

On the other side: If you get into the habit of "Reviews" on GitHub, which are in the free part, too, GitHub gives you a quick button for "Review commits since your last Review" under the Commits dropdown in the Files view. That mostly only works if you add commits rather than rebase, hence the complaints about contributing to "diff soup", but it's a reasonably useful workflow and there are workarounds on the "other side" to help deal with "diff soup".

This is why some encourage Squash Merging as the GitHub preferred merge button. Review as a bunch of small commits over time, merge an entire PR to a single final commit.

That said, as an alternative to squash merging, git itself provides some useful tools for dealing with "diff soup" style repositories using real merge commits: `--first-parent`. `git log --first-parent`/`git blame --first-parent`/`git bisect --first-parent` and more give you a "PR top-level view" in your integration branches (such as main branch) without you needing to rebase/squash.

I wish more UIs took a `--first-parent` by default approach, including/especially GitHub's weak commit views (though it is understandable why GitHub pushes you to wanting to use its PRs list instead of commit views by keeping them weak).

Re: Some of us like "interdiff" code review

#120

Post by loeg »

This workflow is exactly what Phabricator[1] facilitates, for what it's worth. Also, if I remember correctly, ReviewBoard. (Though I have not used that in some time and might misremember. Also, it has its own flaws.)

Sadly the open source version is unmaintained now. It is still used by FreeBSD. Facebook uses it internally for every single diff, of millions.

[1]: https://secure.phabricator.com/book/phabricator/article/diff...

Post reply on HN