I prefer one commit to main per feature, a long with a good description on the GitHub PR.
Sometimes I’ll branch out from a feature branch for the occasional and infamous ‘get CI working’ round of 10 one-line commits though, to not make it too muddy.
Several reasons:
* facilitates much better code review discussions
* enables use of git bisect to locate bugs
* allows for informative commit messages associated with the changes
* communicates clearly to future self about why changes were madeThis is really only viable if each intermediate commit on a development branch is intended to be bug free. If that's the standard you and your team work with, that's fine, but it's not usually my standard; in a development branch, I may commit things that don't even compile, let alone work, if it's a good point to commit.
That said, I've also come to the conclusion there's basically two classes of Git users: people who really understand Git and use it fully, and those of us who basically use it as a place to shove source code before quitting for the night.
At one company with a Giant Custom Enterprise App, I ended up occasionally acting as a historian for pieces of the company with bad communication/institutional-memory, ex: "Oh, the +5% Foo charge was because of a request 3 years ago by vice-president X, here's the ticket number, before that it used to be +3%."
In those circumstances--where the implementation is the source of truth for business process--a well-maintained stream of commit-messages become quite useful.
Either a short quip, doc string, or link to the full story is an accessible combo. Nothing is the correct choice for unsurprising code.
Noting each feature ticket that ever affected a line--or even just at the function level--is sort of like maintaining a few thousand incomplete micro-changelogs. Doing it "acceptably well" takes much more effort than grooming the commit history so that someone can click "show change history for selected lines" in their IDE.
Plus consider all the unnecessary noise it makes for people reading the code, or reviewing a PR.
So, yes seeing e.g. how "CSV export for class A" is implemented is a great guide for implementing "CSV export for class B".
Everything you need is in the most recent copy. Showing an old one invites errors for no benefit.
You would then typically supplement reading the commit with reading the current version of the affected code, but looking at the commit points you in the direction of the files and methods you need to look at.
Not to mention just finding the right commit months later sounds like more work than that already.
Personally, even if I were to make the history absolutely perfect, I never get the code right (interfaces etc), the first time. It might be hours or days before I'm 98% happy with the final implementation. Sometimes big refactor opportunities come to me months later, e.g. where I move code needed multiple times into a more central mixin location.
In other news, our developers also create several small PRs every day but each is for an incomplete change that doesn’t stand alone so we’re never quite sure which features are finished in any given build, everyone keeps complaining about being interrupted to do code reviews all the time when the code reviews have no value anyway because they always just say LGTM :+1:, and we have targets that no more than 15% of commits should break production when CI/CD deploys them and that we recover fully within an hour each time that happens. If only there were something we could do to improve all this…
This too, but there's another thing at play as well: many developers don't know git at all. They just memorized enough commands to let them do their work. They don't understand what they're doing, so they can't reap the benefits of the tool they use. You won't get much use of RAW photos if all you can do in a graphics editor is clicking "auto enhance" button.
He was lead and could influence salaries, his opinion mattered. So, in real life, people rarely pushed back. That is not the same as us sharing the same opinions tho. I became more verbal about history not being useful online.
I rarely use history and prefer merge/squash, with automated CI tools, and tests. "Why" is kept in doc strings, comments, specs, and story tickets. Everything viewable in gitlab with automatic links. All this gets out of the critical path, every day.
I submit that, if your code is so complex that diagnosing a bug is a major research project rather than moving forward with a few extra/modified lines of obvious fix, then that is the problem to focus on.
Sorry, but I don’t buy that. By the same principle, there’s also no point in writing unit tests or defining static types or having code reviews, all of which require thought and extra work, yet can yield considerable dividends when done even moderately well.
I rarely use history and prefer merge/squash, with automated CI tools, and tests. "Why" is kept in doc strings, comments, specs, and story tickets.
The argument for a tidy history isn’t just about a different place to explain a change. It’s about presenting work in clearly defined, meaningful steps to other readers like code reviewers, or perhaps someone who found these commits later through `git blame` on a problematic line of code or `git bisect` after a regression. It’s about each commit representing a complete, self-contained change that could later be reverted, or cherry-picked or merged to another branch.
I submit that, if your code is so complex that diagnosing a bug is a major research project rather than moving forward with a few extra/modified lines of obvious fix, then that is the problem to focus on.
Some problems have a lot of essential complexity. The code to solve them necessarily has at least the same degree of complexity. Sooner or later, there will probably be a change to that code with an unintended consequence for something else. Keeping the code and its history tidy and systematic is, IMHO, how you avoid those investigations becoming major research projects.
As an industry we get paid primarily for 1) working software and 2) communicating with stakeholders.
Tidy yet inaccessible (to non-dev) construction stories are not on that path. I would argue unit tests et al are, to ensure #1.
No stakeholders? Put why into a readme, where it can be seen at a glance. Comments can reference docs.
Complexity must be broken down into bite-sized chunks for a solution to be feasible in the first place, reliable in the second. i.e. skull-size limits. If there’s any code I don’t understand I rewrite it until I can. With tests of course.
I see version history as an asset, just like the code itself, tests, developer documentation, the bug tracker database… None of these things are directly visible to end users under normal circumstances, but they are useful sources of information and organisation and collaboration that help developers to create the software that users do see.
To me, a repo with a messy version history is like code full of superficial comments, a test suite with high coverage metrics that still doesn’t exercise the most important functionality, a dev team where the only documentation is some auto-generated static site that reproduces what any decent IDE would show in real time anyway, or a tracker where all the tickets are vague one-liners. You can produce useful software despite those things, but why would you?
Also, I said/meant stakeholders not end-users. Ours definitely do write bugs, look at docs and generate db reports etc.
The main distinction is that things on that list have a high cost-to-benefit ratio to goals 1 & 2, where history maintenance does not. The cost is high and utilization isn't. Additionally it can't be used to communicate with anyone but developers.
Not true. I do not do those to have nice clean process. I do unit tests, because without them the code is unstable and it is hard to fix bugs without causing unrelated ones. If the code is super simple and unlikely to break, I don't do test. I like to use static types, because I am much faster when writing them. The code is more readable and I have less bugs. Now, I have seen both useless and useful code reviews.
But, in all of those cases, things are done because they beneficial impact in final code and speed of delivery. Beautiful git history does not have such tangible measurable benefit. Git blame and bissect work without it, you just need one more step once in a while.
I respectfully disagree. In my experience, a tidy history directly benefits both efficiency and outcomes of code reviews, speeds up investigations of both bug reports and sometimes general background before starting new development, makes development much easier in situations where changes may need to be isolated and deployed to specific environments (not all software is a web app using CI/CD…), makes it much easier to back out a problematic change without causing unnecessary collateral damage, and helps to verify which development has actually been completed and deployed to which stages/environments, which can be useful for general awareness around the team but is particularly important if you’re operating in any kind of regulated field. All of that in exchange for usually spending less time in `git rebase -i` than it’s taken me to write this comment seems like a bargain to me, but YMMV.
Not to mention quality is significantly higher now so you wouldn't want to refer to a granular history of crap anyway. Any time spent on that would have been completely wasted as I wipe out a thousand line file for a new one with a hundred lines because requests hadn't been invented yet and the original implementer didn't understand network protocols or how to use argparse and implemented it from scratch poorly.
If you can afford your first instinct to be reimplementing things from scratch, your understanding of the value provided by proper version control will be limited. Some of us work with constantly changing code developed by thousands of people from all around the world in projects that 15 years ago were migrating to git and that have tons of downstreams, and are thankful for maintainers and processes that keep their commit graphs useful.
Though that said, once you're comfortable enough with git you'll be thanking yourself for commit hygiene even when coming back to your few years old single-person codebases.
In my experience, developer's work consists mostly of gaining understanding of codebases. It's like being a detective. Writing new code happens too, but not as often and it's not as impactful (and usually can and should be handled by less experienced devs wherever possible). Among the most impactful things are single line changes that took a week to write, or a few dozen lines that took months. Rewriting existing code from scratch is something that happens only as a last resort and after very careful consideration. Maintaining some basic version control hygiene makes a whole world of difference in such work. Sure, you can live without it, but you can also live without docs, comments or tests (and sometimes have to - which makes you appreciate them when they're there).
IMHO the expectation is that each commit would 100% pass CI, so if you decided to extract some commits and merge that early you can. This is especially useful when a 6 commit PR is reviewed, and the first 3 commits are fine but there is more feedback on the last three. The reviewer can split the first 3 good ones out, get them merged and whittle down the PR to the remaining three. The subsequent follow up will be less.
IME team velocity goes up with this too, and it encourages small and easy to review commits like a Remove to be extracted and merged early.
Since PRs are always as large or larger than commits, I would much rather have a specific commit flagged than have to wade through the whole PR diff. If the PR is not familiar to me, I want to increase my effectiveness narrowing down the cause, so I can fix it faster.
Aside, `git commit --fixup HEAD` is often better than `git commit -m "oops, one more thing"`, since it means you can easily `git rebase -i --autosquash`.
What I push out are atomic commits that make sense logically, not an external undo log of my text editor; squashing those on merge provides no benefit and only loses useful information. Squashing should happen before push, not on merge, and there's no reason to have buggy "intermediate" commits recorded in your central remote branch at all.
> This is really only viable if each intermediate commit on a development branch is intended to be bug free.
git rebase has an --exec option that allows you to run a command or set of commands for each commit in the branch. You could rebase your development branch before pushing it up for review and ensure each commit passes coffee linting and tests.
> facilitates much better code review discussions
This can be done while adding code to the feature branch
> allows for informative commit messages associated with the changes
I'm assuming you consider individual commits to be the basic unit of change? This isn't always the case. Some products are not amenable to adding features fractionally
> communicates clearly to future self about why changes were made
You can do that with a squash-merge too!
I've noticed people who work on an evergreen deployment can afford to work on a very granular, commit-level. However, if you have to support multiple production branches concurrently and often have to cherry-pick features and fixes across them, features will naturally become the basic unit of change you will find yourself gravitating towards, and will liberally use squash-merging just to keep your sanity.
Hmm, I usually mark PR’s as draft until ready for review, and then I expect the discussion to be about the current state, not a previous intermediate state. Easiest with small PR’s.
> enables use of git bisect to locate bugs
Interesting. I know _of_ git bisect, but haven’t used it as part of my workflow. Have you found it useful to bisect commits on a feature branch (which, presumably, represents unfinished work)?
> allows for informative commit messages associated with the changes
I find using the PR title and accompanying info in GitHub or similar to be quite informative - that should convey the purpose of the change.
> communicates clearly to future self about why changes were made
See above. Perhaps we work differently, but I find it clearer to read a git history where each commit represents a single, complete feature/fix/refactor instead of intermediate steps.
Agreed, it has been standard at most shops I've worked in the past 8-10 years.
Squash-merge is a scourge. I've seen squash merged commits 30 lines long ("try 15", empty line, "try 14", empty line...). I'm not even sure if you can do anything about such commits because squash-merge is a github/gitlab thing. So, I'm not sure if there are hooks to block it via a commit message linter.
And I've seen people going through some intense mental gymnastics to justify avoiding squashing locally, writing a proper commit message and then merging.