6 ms·

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 c

by Vinnl 2y ago

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 <hash of commit that I want to update>`. I'll then push that up and leave a comment reply to the review feedback sharing the fixup commit hash. Then when the PR is approved and I'm about to merge, I'll do git rebase --interactive origin/main --autosquash This will then combine the fixup commits with the correct original commits. I then do a final `git push --force-with-lease` and merge it. (Make sure to note force push before the review is done, because then reviewers lose the ability to see what you added since their last review.) This relies heavily on autocomplete in my terminal, so that I only have to type `git re<right arrow>` to get to that long command above, for example. And it's a bit clunky, so using a tool that supports and encourages this workflow would be nice. But given that I'm stuck with GitHub, it's OK.

Piraty 2y ago
https://news.ycombinator.com/item?id=37086022 https://news.ycombinator.com/item?id=37086022
- tome 2y ago
  Wow, my jaw is on the floor. This is such a good idea! I already had the idea of tracking issues in the same repo as the code -- not that I actually use that idea -- but I didn't have the idea of doing PRs in the repo itself. Love it!
  - Piraty 2y ago
    tracking issues in the repo would work well. for example, have a dir like ./issues/fix-this-weird-bug.md , and once fixed rename it to ./changelog/fix-this-weird-bug.md . if you adhere to some rules for the metadata, you can use this to base your per-release changelog off of it (some tools already do similar)
mizzao 2y ago
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)
- aseipp 2y ago
  Interactive rebase is fine, I used it probably 50x a day for many years. The problem isn't really with Git here, it's more a bunch of interrelated problems with GitHub's UX, and the fact that many people culturally have never approached the problem in any other way. A lot of places basically just outright copy GitHub's UX, which I think is a good example of this. For example, using 'git absorb' or autosquash doesn't solve the problem of the review UX just doing badly when you say "What is the difference between the last version of commit Y, and the new version." If those changes include a rebase, it shows you the entire diff including the rebase. That's often not good, but sometimes extremely necessary. (See my other posts in the thread about how Gerrit does this better.) And if you have to do something like rebase, then solve a conflict as a result of the rebase, it is basically unavoidable that you have to do all of them at once if you want the CI to stay clean (which I find important, at least.) Also, a lot of teams just culturally hate shit like autosquash, or any squash/rebasing at all. Do I think they're misinformed? Yes. Do I think they could use tools that make those flows 1000x better and faster? Yes. But at the end of the day default user interface and design philosophy of one of the most popular software forges in the world does actually matter and have downstream cultural impact. So you have to interrogate that and break it down for people. GitHub does not really encourage, explain, or smooth out workflows like this. People learn it, then hate anything else. (I mean, fucking hell, Git rebase didn't even have --update-refs, a huge QOL improvement for stacked diffs, until like 2022!) So, you often don't have a choice. I've had many places where engineers despised GH force pushes, but only because... GitHub's UX is bad at tracking addressed/reviewed comments on a PR after a force push! They get hung by their own noose, so to speak, and don't even know it. I do have other problems with Git, but this post is mostly about GitHub/Gitlab style PR review flows. The same problems still happen even with autosquash or tools that accomplish the same thing.
- jrochkind1 2y ago
aseipp 2y ago
Yes, fixup commits are a good way to approach this, though I don't personally like them, though. I think Sapling's "absorb" command which works on an underlying SCCS weave to automatically absorb changes into the relevant diff is much more elegant. It prompts you with an interactive UI. (For me, it sorta falls into the same space as rebasing a series that has multiple branches on it. You need --update-refs for that. Because otherwise it's like, why am I, the human, doing the work of tracking the graph relationships and manually punching in the commits, and moving the branches, and doing all this bullshit? Computers are good at graphs! Let them handle it!) There is also a `git absorb`, but it isn't as robust as Sapling's implementation[1]. Really the problem isn't interactive rebase or not. It's mostly a problem of the UX of the review tool itself more than anything, and the kind of "cycle" it promotes. I mentioned it elsewhere here, but fixup commits for example still won't solve the problem of GitHub showing you diffs between baselines, for example, which can absolutely ruin a review if the baseline is large (e.g. you rebased on top of 10 new commits.) I do have problems with Git's UX beyond this, but the original post is mostly a gripe about GitHub. [1] There is an example in this GitHub issue that captures the difference between the two underlying algorithms: https://github.com/martinvonz/jj/issues/170 https://github.com/martinvonz/jj/issues/170
- Vinnl 2y ago
  I really want to give Sapling a try one day, yeah. Note that the workflow I described is indeed mostly a workaround for people, like me, stuck with GitHub.
faangguyindia 2y ago
Biggest problem working with Git is remembering all these flags and commands. If you do it everyday for years, sure you remember most of them. But for weekend hackers or those who specialise in some other areas, it creates a lot of frustration. I often forget all these flags etc.... My brother made this https://github.com/zerocorebeta/Option-K https://github.com/zerocorebeta/Option-K This enables me to simply write in thermal, "interactive rebase main, auto squash. And then I hit option+k and it replaces it with the above command.
- ipaddr 2y ago
  If you are a weekend warrior you are not working with a team. Just commit and push.
- keybored 2y ago
  You’ve only mentioned your bros GitHub projects four times in the last two days. Time to step it up maybe?
riquito 2y ago
You can add to your ~/.gitconfig [rebase] autosquash = true from then on `git rebase -i origin/main` automatically reorder fixup and squash commits. It's a small thing but greatly improved my workflow
- dietr1ch 2y ago
  Squashing is nice because it keeps the amount of commits minimal, but as mentioned in the article, it has the problem that now reviewers have to figure out what changed since their last review, which can get hard to do if the tree of changes originally proposed needed updates in dependencies of the commits they reviewed. I really like the idea that I'm working on a tree (often a stack/list) of changes, and as the review progress I get this cross product of iterations over time like [a-better-way-interdiff-review-aka-git-range-diff](https://gist.github.com/thoughtpolice/9c45287550a56b2047c6311fbadebed2#a-better-way-interdiff-review-aka-git-range-diff https://gist.github.com/thoughtpolice/9c45287550a56b2047c631...) showcases. In the end, the time dimension is useless after things get submitted, so the repository only gets the latest state of the change tree committed, which is simple and like the auto-squashed version you mention, but during review reviewers get to see the change tree evolve in an easy way.
  - pinkorchid 2y ago
    > the problem that now reviewers have to figure out what changed That's something that the review tool can solve, and I agree that github doesn't handle it well. But other code review tools can show diffs between revisions independently of how the commits have been rebased or squashed over time.
    - dietr1ch 2y ago
      That's part of the problem, if you are a reviewer you'll see the changes that other reviewers asked for on code that you did not want to review. I guess this might be a problem mostly on large projects on monorepos, where different systems that interact end in the same codebase, but there's a cost to be paid too if each system has their own repo, build/test/deploy infra and independent ownership.
Guvante 2y ago
Git aliases are magical BTW

by Vinnl 2y ago

Piraty 2y ago

https://news.ycombinator.com/item?id=37086022 https://news.ycombinator.com/item?id=37086022

tome 2y ago
Wow, my jaw is on the floor. This is such a good idea! I already had the idea of tracking issues in the same repo as the code -- not that I actually use that idea -- but I didn't have the idea of doing PRs in the repo itself. Love it!
- Piraty 2y ago
  tracking issues in the repo would work well. for example, have a dir like ./issues/fix-this-weird-bug.md , and once fixed rename it to ./changelog/fix-this-weird-bug.md . if you adhere to some rules for the metadata, you can use this to base your per-release changelog off of it (some tools already do similar)

mizzao 2y ago

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)

aseipp 2y ago
Interactive rebase is fine, I used it probably 50x a day for many years. The problem isn't really with Git here, it's more a bunch of interrelated problems with GitHub's UX, and the fact that many people culturally have never approached the problem in any other way. A lot of places basically just outright copy GitHub's UX, which I think is a good example of this. For example, using 'git absorb' or autosquash doesn't solve the problem of the review UX just doing badly when you say "What is the difference between the last version of commit Y, and the new version." If those changes include a rebase, it shows you the entire diff including the rebase. That's often not good, but sometimes extremely necessary. (See my other posts in the thread about how Gerrit does this better.) And if you have to do something like rebase, then solve a conflict as a result of the rebase, it is basically unavoidable that you have to do all of them at once if you want the CI to stay clean (which I find important, at least.) Also, a lot of teams just culturally hate shit like autosquash, or any squash/rebasing at all. Do I think they're misinformed? Yes. Do I think they could use tools that make those flows 1000x better and faster? Yes. But at the end of the day default user interface and design philosophy of one of the most popular software forges in the world does actually matter and have downstream cultural impact. So you have to interrogate that and break it down for people. GitHub does not really encourage, explain, or smooth out workflows like this. People learn it, then hate anything else. (I mean, fucking hell, Git rebase didn't even have --update-refs, a huge QOL improvement for stacked diffs, until like 2022!) So, you often don't have a choice. I've had many places where engineers despised GH force pushes, but only because... GitHub's UX is bad at tracking addressed/reviewed comments on a PR after a force push! They get hung by their own noose, so to speak, and don't even know it. I do have other problems with Git, but this post is mostly about GitHub/Gitlab style PR review flows. The same problems still happen even with autosquash or tools that accomplish the same thing.
jrochkind1 2y ago

aseipp 2y ago

Yes, fixup commits are a good way to approach this, though I don't personally like them, though. I think Sapling's "absorb" command which works on an underlying SCCS weave to automatically absorb changes into the relevant diff is much more elegant. It prompts you with an interactive UI. (For me, it sorta falls into the same space as rebasing a series that has multiple branches on it. You need --update-refs for that. Because otherwise it's like, why am I, the human, doing the work of tracking the graph relationships and manually punching in the commits, and moving the branches, and doing all this bullshit? Computers are good at graphs! Let them handle it!) There is also a `git absorb`, but it isn't as robust as Sapling's implementation[1]. Really the problem isn't interactive rebase or not. It's mostly a problem of the UX of the review tool itself more than anything, and the kind of "cycle" it promotes. I mentioned it elsewhere here, but fixup commits for example still won't solve the problem of GitHub showing you diffs between baselines, for example, which can absolutely ruin a review if the baseline is large (e.g. you rebased on top of 10 new commits.) I do have problems with Git's UX beyond this, but the original post is mostly a gripe about GitHub. [1] There is an example in this GitHub issue that captures the difference between the two underlying algorithms: https://github.com/martinvonz/jj/issues/170 https://github.com/martinvonz/jj/issues/170

Vinnl 2y ago
I really want to give Sapling a try one day, yeah. Note that the workflow I described is indeed mostly a workaround for people, like me, stuck with GitHub.

faangguyindia 2y ago

Biggest problem working with Git is remembering all these flags and commands. If you do it everyday for years, sure you remember most of them. But for weekend hackers or those who specialise in some other areas, it creates a lot of frustration. I often forget all these flags etc.... My brother made this https://github.com/zerocorebeta/Option-K https://github.com/zerocorebeta/Option-K This enables me to simply write in thermal, "interactive rebase main, auto squash. And then I hit option+k and it replaces it with the above command.

ipaddr 2y ago
If you are a weekend warrior you are not working with a team. Just commit and push.
keybored 2y ago
You’ve only mentioned your bros GitHub projects four times in the last two days. Time to step it up maybe?

riquito 2y ago

You can add to your ~/.gitconfig [rebase] autosquash = true from then on `git rebase -i origin/main` automatically reorder fixup and squash commits. It's a small thing but greatly improved my workflow

dietr1ch 2y ago
Squashing is nice because it keeps the amount of commits minimal, but as mentioned in the article, it has the problem that now reviewers have to figure out what changed since their last review, which can get hard to do if the tree of changes originally proposed needed updates in dependencies of the commits they reviewed. I really like the idea that I'm working on a tree (often a stack/list) of changes, and as the review progress I get this cross product of iterations over time like [a-better-way-interdiff-review-aka-git-range-diff](https://gist.github.com/thoughtpolice/9c45287550a56b2047c6311fbadebed2#a-better-way-interdiff-review-aka-git-range-diff https://gist.github.com/thoughtpolice/9c45287550a56b2047c631...) showcases. In the end, the time dimension is useless after things get submitted, so the repository only gets the latest state of the change tree committed, which is simple and like the auto-squashed version you mention, but during review reviewers get to see the change tree evolve in an easy way.
- pinkorchid 2y ago
  > the problem that now reviewers have to figure out what changed That's something that the review tool can solve, and I agree that github doesn't handle it well. But other code review tools can show diffs between revisions independently of how the commits have been rebased or squashed over time.
  - dietr1ch 2y ago
    That's part of the problem, if you are a reviewer you'll see the changes that other reviewers asked for on code that you did not want to review. I guess this might be a problem mostly on large projects on monorepos, where different systems that interact end in the same codebase, but there's a cost to be paid too if each system has their own repo, build/test/deploy infra and independent ownership.

Guvante 2y ago

Git aliases are magical BTW