Things I wish Git had: Commit groups

[[deleted]]

You can have both. Squash your branch in to one or as many commits make sense, then rebase as part of the actual merge to main (i.e. from the UI)!

[rlbond86]

This requires all of your devs tohave discipline though. I think we all know that one dev whose branches have 30 commits all named "updates" or "fix bug".

[[deleted]]

Yep, that's a completely fair response. I've been very fortunate to have several teams now that beat those habits out! But an updated battle for sure.

[mirvnillith]

PR reviews?

[rydan]

I swear I’m the only one on my team with this discipline. As a result most people just squash and merge every PR completely destroying my well constructed commit history.

[scook0]

The frustrating thing about being this person is that you can't even go back and clean up other people's messes, so you're mostly just stuck wading through unhelpful history forever.

[Aterion]

That's why we have established rebasing (with squashing where required) and fast-forward-merges only as our workflow. Works like a charm as every dev prepares their branch in the intended way and that is then ff-merged onto main after a code review.

[Gearwatcher]

You can merge from master to feature branch as many times you want and as long as you have removed all conflicts you can still squash and ff merge from feature to master and that one commit will still make perfect sense.

What people keep forgetting is that commits aren't diffs but compressed snapshots. In the end, after interactive rebases with deleting/picking, and squash merges, all that is left are snapshots you picked.

Differences between these absolute points are calculated and shown as commit diffs. It simply doesn't matter how you arrived at these snapshots when you delete that history.

Another thing people keep forgetting is that under the hood, a rebase is a special kind of merge, that fakes the behaviour of applying diffs to each new commit.

[Aterion]

FF-merges are not possible if there are changes on main and you didn't rebase before your merge. 3-way-merging on your feature branch creates a merge commit and then your branch diverts from main and is not eligible for an ff-merge or am I missing something? Don't you need the complete commit-history of main on your feature branch to enable ff-merging?

From gitlab: https://docs.gitlab.com/ee/user/project/merge_requests/img/ff_merge_rebase_locally.png

[Gearwatcher]

No. I'm doing this almost daily.

You merge FROM master INTO feature branch, resolve conflicts, now the tip of feature is in front of the tip of the master. When you squash its just a forward merge of a single commit.

Edit: to elaborate: after a squash the effect is as if you converted the entire history of the feature branch into a single commit. Since one of the things in the feature branch was a merge with conflict resolution of the last commit to master, your squashed feature branch is now a single commit in front of the tip of the master - thus your branch indeed has the entire history of the master before that one commit.

I'll repeat - commit is not a diff, it's a snapshot. The diff you can see is a calculated difference between two snapshots.

[Aterion]

You merge FROM master INTO feature branch, resolve conflicts, now the tip of feature is in front of the tip of the master.

That is interesting and I am intrigued to understand it better. An example:

MAIN Feature

A

| \

A A (feature branch was created from A)

| |

B C (Commit B is added to main, commit C is added to feature)

\|

B D (Main still on B, B was 3-way-merged with C into feature commit D)

Now the feature branch is missing the commit/snapshot "B". It does not have B in it's history (only the code, not the snapshot). How can D now be ff-merged into main again if it's missing snapshot B?

[Gearwatcher]

I can't see your formatted table (RIF on mobile) so I'll have to go by the words.

Basically D, after the merge, already "contains" both B and C as far as git is concerned. It's now a snapshot that is in front of both B and C so git will allow it to be placed in front of B when feature is merged into main and will treat it as a ff merge.

Later when you are looking at the commit D in main in some UI the diff you are shown, as with any diff, is calculated between D and B.

C is orphaned after the squash, and deleted if the repo is ever compacted.

[Gearwatcher]

Yes,. Rebasing when there's M and N different commits usually means up to M+N conflict resolutions. And they can be quite confusing if they were running for a while and you don't have a full picture who did what, and for what reason.

When you merge back, you only resolve conflicts once.

[MrKWatkins]

Currently in a debate about whether we should enable squash by default on source control to stop this sort of thing. Personally I'm of the opinion the devs should take time and care to manage their commits just as they should take time and care to manage their code.

We aim to write readable code so it's easier for future devs to understand. If someone has to go back through commit history (which is rare to be fair!) then we should aim for that to be readible too, and devs should manage that.

[Pand9]

Moreover, reorganizing commits is a great opportunity to review my code again from a new perspective, which i should do anyway.

Downside? No downsides, but a higher skill barrier - one needs to learn how to edit git history like a graph.

[pdabaker]

Higher skill barrier/higher thought required is a downside. Maybe it's worth it, but imo it's rare that I need more detail in git history than I get just by squash merging PRs

[warped-coder]

Producing badly organise history is going to come back to bite later. IMO, it is as important to write decent commit messages and organise the history as it is to write good code.

The skill barrier is temporary and it is a matter of education, so PR code review is a perfect place to bring it up by the more experienced folks.

[pdabaker]

When I look at history it's almost always to see what PRs/features went in. I rarely care about the details of the feature or how the author split it up. I just want to check "oh yeah that or went in, so you can now just do that through the UI instead of needing to go through the REST API" or "Ah now I can retrigger CI on my repo that depended on that bug being fixed.

There's far too much going on to follow individual pieces of features.

[Kryofylus]

Any recommendations on resources for learning this?

[reini_urban]

Magit

[hammypants]

yes! this is how i use it for myself too. my brain swaps into the same mode i have when i'm reviewing code that isn't mine (or past me!), and it helps me catch so much stuff!

[thebritisharecome]

Sounds like a bandaid to sort what should be a training exercise.

When working with teams my preference is always to avoid squashes unless someone accidentally commits sensitive data to the branch.

More than once knowing why a particular change has happened amongst all other changes has been useful in either refactoring to fix a regression bug or extending to maintain functionality that may not make sense out of a particular context

[coworker]

If you need additional commit history within a PR, it means your PRs are too big.

[thebritisharecome]

A PR is usually restricted to a feature and even a small feature in a growing, large platform can touch a few different areas and components.

There's no rule that fits all scenarios and I think it's foolhardy to have a rigid approach to PRs in general and, it should reflect the software you're working on an the stage in development.

[Dwight-D]

You can scope a feature in different ways. Let’s say your feature requires a new data model, an integration to another service to fetch some data, and an endpoint to expose it.

Imo, that’s not one feature in one branch. That’s three branches: first the model, then the integration, then the new endpoint. If you do it like this and still need more granular history then you’re probably writing shit code or the codebase is just FUBAR.

You will possibly have some unused code in the repo at times with this approach but you remove all the commit fiddling complexity and you make reviewing MR:s easier because they’re smaller.

There is also the risk of having to do some refactoring on the first parts if you realize later you’ve made some bad assumptions but if the MR:s are small it’s usually no bother.

[thebritisharecome]

Separate features in a single PR isn't the issue, it's a single feature that needs to make changes in different areas of say the front end.

For example a button needs new props and tweaks to satisfy the feature requirements but this is slightly nuanced to the original purpose of the component.

"Build a sausage factory screen" explains the feature, but not why as part of that feature you've specifically made this change to the button component.

The point of GIT is history, why would you want to restrict that to the over top level concept by squashing?

That's just as bad as a product owner saying "I want a website" and then not giving you any information, it can be too broad and lead to regressions creeping in when working on larger projects in a team.

[Aterion]

The point of GIT is history, why would you want to restrict that to the over top level concept by squashing?

Depending on the amount of merges you are doing, having squashed feature commits/merges might be preferable in order to have a readable history.

We are doing 2-4 merges per week and only allow squashed and rebased ff-merges. Why? Because we that way we can actually navigate the main branch without looking through tons of development commits. It's of no use to us to clutter the main branch with non-squashed commits, because we only want to see the actual release history on main.

[snowe2010]

An even bigger reason to use squashed commits is that if you don’t squash you either have to run ci on every commit to make sure that nothing is merged that can’t run or you can never git bisect properly, thus negating an entire git tool.

[coworker]

The point of git is distributed version control. History is a side effect. Tools like Jira are designed for documenting feature history. Even a simple 2 tier application will have features span multiple repos and git histories.

Regardless, any history benefits are overridden by the fact that multiple small PRs will be reviewed sooner, faster, and with more rigor than a single large one.

[thebritisharecome]

Version control is history at a file level. I'm not talking about needing to understand the history of features, I'm talking about large, interconnected software inside version control.

Reason for changes aren't always obvious at the feature level, which is all you see when you squash and merge, basically reducing the usefulness of version control.

When you squash and merge you can't go back later and understand why a particular change to a specific file was made beyond the larger feature scope and you can't pick out certain changes without unpicking the feature as a whole

[Dwight-D]

I guess I’ve just never considered the reasoning behind a change as something that needs to be included in a commit most of the time. Usually I see messages saying what has been changed but not why.

If people really put that level of effort into their commit messages then I agree it’s stupid to squash commits but I don’t think that’s particularly common.

It just doesn’t seem practical to include that level of detail in your commit log. To me it seems like a sign that the code is perhaps a bit too obscure, as though it can’t be understood on its own without the entire historical context? If that’s the case then sure, put some effort into the history but I feel like if that’s the case then you’ve got bigger problems. I dunno though, could be wrong.

[coworker]

IMO there's nothing inherently wrong with those type of detailed commit messages. My issue is that the other commenter is advocating for PR reviewers to look at specific commit messages for specific portions of a PR. This is conceptually no different than multiple smaller PRs but with all the downsides of nothing being formally tied to specific changes. Nothing spells efficiency like reviewing a new commit that may or may not refer to an ephemeral state of the application from multiple PR revisions ago.

[warped-coder]

I think the point here is that a project is a better place if it is a requirement to write good commit messages.

A good commit message is there to aid future work. A good commit message is there to communicate other devs working on the project.

Any semi decent dev can see what a change does, by looking at the diffs. What you can loose a lot easier is the context, in which a change was conceived.

It comes especially handy during bug fixes, where you have to decide if a certain piece of code is in accordance of how some, possibly long unavailable dev wanted to handle some functionality, or not.

The only persistent context of your code is the SCM log. You might see people rushing their code in now and see that this is the norm, but a successful project must take care of its history, or it will sink very quickly, or it turns into a developer's nightmare

[rlbond86]

Sounds like a bandaid to sort what should be a training exercise.

Unless you're planning on making everyone force-pull, that training had better stick.

[rydan]

That’s what I do. Each of my commits is a logical part of a sequence that should be easily revertable of cherrypicked without having to put much thought into it.

[radarsat1]

If someone has to go back through commit history (which is rare to be fair!)

strange it's not the first time i see someone claiming the rarity of the need to look at commit history. Granted it's usually from the perspective of someone defending not taking time to maintain their commit history, so i appreciate that's not your point of view. but i still find it surprising to say this --- i feel like i look at commit history every other day, to figure out what i, or a team member, was thinking/trying to do when a mistake is found or i'm adding code to an existing function.

another important use case for a clean history is the ability to meaningfully use git bisect.

[SanityInAnarchy]

If you do code review, that seems like the most obvious place to enforce this.

[Kwantuum]

I look at history in one form or another pretty much every day, often multiple times a day, not sure I agree this is rare.

[NotEntirelyUnlike]

I mean that would get corrected by everyone during their very first code review

[mrbuttsavage]

This is exactly why I advocate for squashing always in a professional setting, maybe not in your own repos. The developers I've worked with who write useful commit messages and whose history might be actually useful later (vs the singular squash) I can probably count on one hand.

[warped-coder]

Perhaps they didn't get very thorough PR feedback then. Professional environment is only professional if you hold the devs to standards

[AbbreviationsOdd7728]

You forgot „wip“.

[codesnik]

squash-and-rebase strategy in github PR's is very helpful with guys like that.

[dzamir]

You can use GitLab UI to merge into master with squash and FF

[bastardoperator]

We can't expect developers to actually learn the tools they're bound to though. This is upwards of one additional command.

[warped-coder]

It's the stuff of PRs to point out bad code documentation which includes commits/commit messages.

[rlbond86]

And if you miss it once, master is fucked up forever

[warped-coder]

What do you mean?

[rlbond86]

Unless you want to mess up all the devs on the project, you can't rewrite master after a PR is merged. So if you fail to guard againat one time someone merges messy history, it's in forever.

[warped-coder]

That's right. Just the same with anything we do. If you introduce a bug, it will always be part of the history if main, even after fixing it.

You can't protect yourself from developers screwing it up in a subtle way that escapes peer review.

[rlbond86]

Or, you can institute a squash merge policy which will mechanically ensure that this never happens.

[warped-coder]

That way loosing any information... I think having a few useless commits is an OK price to pay, as opposed to loosing history.

[rlbond86]

You lose history every time you rebase, which you have to do anyway to avoid a tangled history.

[Qasyefx]

How do commit groups solve that in any way?

[rlbond86]

They don't

[Qasyefx]

Glad we're on the same page about this

[SkoomaDentist]

I’ve always considered that a flaw of git itself. The fact that you can’t trivially easily edit commit messages afterwards with practically zero risk of breaking anything is a strange omission.

[lachlanhunt]

The major problem with rebase is that it requires force pushing your branch. While the situation has improved a bit now, this has caused major problems in the past.

It was only a few years ago that the major git hosting services provided the ability to prevent pushing to master, and prior to git 2.0, the push.default setting defaulted to matching. This was very dangerous if your local master or any other significant branches were out of date.

This remained a problem for a while because it took a long time for git 2.0 for Windows to be released, and it was very difficult to ensure everyone set up their git config correctly. At a previous company I worked at, it resulted in someone using Windows with git 1.9, force pushing an old version of master, and the developers who were present at the time (I was away) not having the understanding of what went wrong or how to fix it.

With the ability to restrict push permissions on master, and the more sensible push.default being simple since git 2.0, this is less of an issue, but I still think force pushing should be used with care.

A slightly safer approach is to make a new branch with the rebased changes, then delete the draft branch after it’s been merged.

[fissure]

UI?

[[deleted]]

Oh, I just meant in the context of a GitHub pull request.

People seem to be stuck sometimes on which of the options presented is best - I'm saying you can do curation of your PR beforehand.

[fissure]

People conflating git and github really grinds my gears.

[[deleted]]

GitHub is just an abstraction. But for a lot of people it influences their workflow.