I'm comfortable git fooing w/e is necessary, but ever since we adopted this, git related conversations went to almost zero. It's great.
The notable features are: - Move commits up and down, fixup, drop - Rename commits from the editor (without having to stop for a reword during the rebase run) - Visualize modified files along commits - 'Explode' a commit ,creating a commit for each modified file (a thing I found myself doing quite often)
Feedbacks (both on the tool and the code) and contributions welcome, hope it could fit other people needs too !
I'm comfortable git fooing w/e is necessary, but ever since we adopted this, git related conversations went to almost zero. It's great.
The PR is the unit of work, people get too hung up on the PR's individual commits.
Worst: rebase and merge - you end up with your coworker's broken WIP commits all over master, and have a terrible git revert story
OK: merge commit - you can revert, but there is a less intuitive `-m 1` flag (IIRC?) you have to pass into revert, and IMO you rarely need the intermediate history.
Best: squash & merge - you get one single commit representing the unit of work merging in, git revert is dead easy
Also setting the commit message in main to the `{title} (#{prNum})\n\n{prDescription}` format preserves all the good context from your PR and lets you get back to it if you need.
IMHO the guideline should be "clean up your branch and rebase it before merging. Usually that means a single commit, but it can be multiple if that makes more sense to <future person> reading the history".
Commit history on a (larger?) PR tends to be most useful during the PR itself; I tend try and make my commits tell a story I can walk people through (on the CLI during a call) moreso than anything that will be useful in 6mo/year.
I've been convinced on Squash/Merge. If the PR needs more granular commits; maybe it should be 2 independent PRs.
1. You get rid of WIP commits, typo fix commits, all kinds of transient changes (for review and so on)
2. You keep the substantive ones
3. All changes are logically separated
I don’t see where you cover this option.
> Also setting the commit message in main to the `{title} (#{prNum})\n\n{prDescription}` format preserves all the good context from your PR and lets you get back to it if you need.
Again. It is ironic that people are so comfortable with dumping the history in a webapp when you are working with a version control system.
I am fine with a lot of the history being on GitHub or whatever other webapp. The review history, that is. But I’m not comfortable with leaving the history of the pre-squashed commits on GitHub if those pre-squashed commits were useful for the long-term history.
In my experience, engineers tend to fall into 1 of 2 camps: 'Deep' Git knowledge who routinely dig through reflog and keep backup branches, commit early/often and autosquash logcal chunks until their PRs tell a Story through their commit history. The other side pretends git is p4; and has no concept of fetch vs pull vs rebase. A base assumption that branches are expensive and to be avoided.
I'd like to think probably fall in the middle, but nearly every engineer I've worked with falls on those edges based on the number of DMs i get asking for help after after the rote `git stash; git pull; git stash pop` throws conflict.
We constantly use that on the OSS project I’m occasionally involved with. “Well why is it like that…” and the project has enforced a good history so this can often be answered.
And that’s the primary benefit of Git. With some discipline the history becomes legible.
On the other hand it doesn’t give you much out of the box for code review. Unless the code review you use is covered by the email tooling.
The problem with the squash strategy is when it is used as a team-wide policy. Like it was described here. I can’t understand why we have to collectively limit ourselves like that.
And if the PR author deems their commits insignificant, they can feel free to make one PR and they squash & merge.
Vanilla tools make PRs have a certain overhead. Dependent branches and flipping from the terminal or IDE to the webapp and so on.
And these vanilla tools are often a team requirement. But with git(1) you can work around all that overhead.
Am I gonna stop weirdly insisting on using the version control system itself for version control instead of latching onto whatever passes for “PR”? Apparently not.
With Git you can work in, well, at least twelve different ways. But people discuss workflows with some built-in assumption that of course everyone else is using it like they are.
The version control system itself doesn't have enough features for collaborative coding. It doesn't have all the discussions and messages exchanged between authors and reviewers. The main task here isn't version control itself but a collaborative social process. The kernel has LKML. The rest of us mostly use GitHub. By making one large PR with distinct commits you force that discussion to be intertwined together. That's why I continue to believe the best way is to make multiple smaller PRs each with one commit (or multiple commits that are squashed upon merge).
FWIW for individual repos I don't even use PRs so the whole issue of how to merge them is moot.
[redacted]
You have to create branch names and keep them in synch. with `git rebase --update-refs` constantly. But worst of all: vanilla forge PRs don’t support it. So you have to thread these dependencies manually instead. What magic eliminates this overhead?
Another nice thing with squashing is that merges into master always look the same regardless of individual engineer workflows.
The point? You’re not making sense.
This is a discussion. This is not your team pow-wow where the boss makes a show of hearing everyone out and then going with what he decided with beforehand. Or whatever your process is.
This right here is a discussion. It makes no sense to pull the “we all wear leotards because that’s the team policy”. That’s not an argument that I can recognize.
You should, in a discussion, make where you come from clear. If three comments in on the topic of X you say “but of course we use Y, how dare you suggest something else than Y?” then you’re not having an argument any more.
I tried out that config for a while but it was too much of a sledgehammer in practice. It rewrote things that I didn’t want.
> And what do you mean vanilla forge PRs don't support it?
Hmm. This is what I would call support:
- Push one time and you get the option from the remote to generate X dependent PRs since the remote sees that you have some branch tip and X-1 ancestor branches which are not in `main`... etc.
I at least haven’t heard of this support in GitHub.
What does not rise to that level: having to create X PRs manually and making sure to target each of them individually.
But apparently the Git experts (or simply the ones who do not suck at it) do this with vanilla tooling. I could very well be missing something here.
> If it's GitHub you simply tell your reviewer to look at the commit diff view not the PR diff view.
You mean a five-commit PR could be a one-commit-per-PR by saying to the reviewer “look at the commit diff”? Then I have no trouble with it.
I want to have the choice to do a true merge or some other strategy. It’s a downside for me personally. Not having a choice doesn’t help me since it’s a meaningful choice in my book.
As a policy. I leave this decision for others. People who prefer can squash-merge all the time if they want to.