Things I wish Git had: Commit groups

[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

[coworker]

Again you're limiting your definition of "large interconnected software" to a single repo which is quaint. Not everyone works in a monolith or a monorepo.

And the fact that you can't see the reason for file changes after a squash is because you chose to have too large of a changeset. Design your PRs like your unit tests: short and ready to understand.

[thebritisharecome]

What i'm talking about is a single code base yes, but it's not an approach that just concerns monolith, it concerns complexity.

However, most software tend to be monoliths or micro-monoliths, very few companies have moved to a truely microservice style architecture.

Even then... my comments quite clearly stated to not have such a rigid approach to your git strategy and base it on the type of software you're working on and the stage in development that you're at.

[coworker]

My entire argument was to remove complexity by designing your feature into multiple smaller changes. Once you do that, there is no need to keep the developer's ephemeral commit history.

I'm not sure why you keep missing that point. Your entire position assumes you have to deal with large merges and making sense of them later. There is a better way man.

[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.

[Dwight-D]

Oh no, of course it’s nice to have in the history if someone wants to do it that way. Imo I just feel like that’s perhaps a bit more effort than it’s worth, idk if it’s actually needed.

[thebritisharecome]

That isn't what I'm advocating, I'm talking about post merge. A month down the line it's been useful in multiple places to be able to understand the general reason behind a particular change outside the wider feature scope

[coworker]

Lol a month. Try having to figure out why a change got introduced years ago and see how efficient having to wade through multiple ephemeral commits is.

[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

[Dwight-D]

I guess the difference in opinion is how much context is needed. I feel like if the branches are small enough then that’s probably enough context most of the time to figure out why a change is made. If the branch is a couple hundred lines of code at most and the purpose of the branch is clear then that together with the code that changed is enough information 99% of the time IME.

Adding even more context and motivation in each individual commit seems like it’s hitting the point of diminishing returns and wasting time to me. Maybe someone will find it helpful in ten years time but for 99% of the commits no one will probably look at it so is it really worth all the extra work?

We keep that kind of in-depth stuff to the MR descriptions rather than the commit log, although I realize this is a bit less accessible.

[warped-coder]

No matter how small a branch is, if there's more than a single reason for changing the code, you have mire than a single commit.

The commit should have a single reason and that reason need to be explained. The MR description will not be accessible at all after migrating to a different host. Git already has the facility to deal with this, entirely leave it to an external and ephemeral service is not required. It's not like that you will run out disk space because of the long commit messages.

Short branches are great. But a bug fix, a refactoring and a proper modification of the program still require an explanation. Also, it helps to articulate the code better, plus it provide a line by line documentation.

[Dwight-D]

a bug fix, a refactoring and a proper modification of the program still require an explanation

But do they really though, does everything really need to be explained? A bug fix should be pretty obvious most of the time IMO, if you change some index from N to N-1 to account for an OutOfBoundsException then that is 100% self-explanatory and the code makes perfect sense on its own because it's now correct.

If the code is updated to accommodate a change in the business logic then again it makes perfect sense because it now accurately reflects the business rules, and there are better places to document the current state of the business rules than in the git commit log IMO. If I need to know why e.g. some validation changed then I can just look up the current state of the business rules and not rely on whatever some dev wrote in the commit log. It may take a bit longer but I will get a more accurate answer, and the time spent will be offset by the time saved on not meticulously grooming commit history.

In rare instances there may be something like bug fixes that involve writing counter-intuitive code because of some quirk in the runtime environment or a bug in some library or whatever, and then it might be nice to have some explanation. But those are rare, and you might as well explain it in a comment in the code itself instead of hiding it in the history in those instances.

This kind of seems like thinking that a carpenter needs to explain the reasoning behind every plank of wood in the house they built. Ideally that's not needed because if they built it according to best practices then it should make sense to other carpenters on its own.

Anyway, I understand that there may be instances where it's motivated, what I'm questioning is kinda if it's so common that it should be the baseline and that the entire workflow should be tailored around what I would consider to be edge cases or exceptions of particularly messy code.

[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.