Hacker News Reader

> Squashing that history of 25 commits, will create one giant

> commit of 1500 changes. Have fun finding where in those

> 1500 changes that little nasty bug was introduced!

100% agree, the only purpose of this is to make "pretty graphs" at the cost of destroying the history of the code changes.

I prefer to write small commits with simple messages that explain exactly what that change does. Sometimes it costs some time in Git Gui selecting the lines that make up the commit, but ultimately it's worth it.

> I prefer to write small commits with simple messages that explain exactly what that change does.

One thing people should include in commit messages is why the change is being made. In other words, what's the purpose of the change. That can provide a lot of context that can be lost over time. So my commit messages include both what was done and why it was done. The why part can range from things like this method is needed for this feature that does something, or this fixes a bug that was causing this issue, or this other feature is no longer needed in context of this other reason, etc.

What can also help is linking to the related Jira ticket (or whichever issue tracking system you use).

And separate "cleanup" commits from commits that make meaningful changes. A commit that changes every line in a file because it's fixing incorrect indenting is fine. A commit that does that but also introduces a small functional change, makes that change invisible.

We've actually changed tracking systems over the years (meaning that the link in the commit message is no longer valid). So, it's best to include both the link and the actual text.

> A commit that does that but also introduces a small functional change, makes that change invisible.

While it's sometimes possible to separate the actual change from the formatting change by ignoring whitespace if the diff, it's definitely better to make each type of change in a separate commit.

Absolutely. I don't recommend replacing the text with the Jira ticket number. List them both. Both are useful.

> One thing people should include in commit messages is why

> the change is being made.

In a perfect world, there would be a massive document with each commit, specifying exactly how it behaves, all the possible edge cases considered, why it was implemented, other implementation choices and a justification for why this one was selected, etc, etc.

I think the reality of the matter boils down to a few points:

1. Commits are not read that often and if they are, it's usually recent history. If they are being read very much in the future, then the "why" may not even make sense either.

2. Unbounded limits on commit message lengths leads to some people waffling in their commits. If you can't explain it in 80 characters, you should very much consider breaking it down.

3. Usually it's not possible to isolate a commit from the context of the commits around it.

4. Usually it's not possible to understand a commit without the larger context of the system.

Generally if some implementation requires an explanation, it can be better understood as a comment next to the implementation in question.

> 1. Commits are not read that often and if they are, it's usually recent history. If they are being read very much in the future, then the "why" may not even make sense either.

It depends. If you're looking at the commit log directly, then you would tend to look at recent history, but if you're using something like git blame, you can easily come across commits that are far less recent.

For example, part of your change is to update a line of code in a file. You run git blame and see the commit that introduced that line of code, run git show for that sha1 and see the context around why that line was introduced (e.g., a bug fix). If you don't look at the history, you could easily introduce a regression in the code without realizing it. In other words, the VCS history can be used as a tool to help determine what potential effects a change could have.

> 2. Unbounded limits on commit message lengths leads to some people waffling in their commits. If you can't explain it in 80 characters, you should very much consider breaking it down.

There's no restriction on how long a commit message should be. Convention from the Linux kernel and git itself says the title should be limited to 50 characters and the body should be wrapped at 72 characters (other than things like links and error messages/log lines). There's no restriction on how long a commit message should be.

As for whether to split the commit up, that really depends on whether you're doing multiple things in a single commit, which may or may not have a relation to how long the commit message is. Typically, if you see something like "do A and do B", then that's a sign that the commit should be split into separate ones where one does thing A and the other does thing B.

> Usually it's not possible to isolate a commit from the context of the commits around it.

If you use a combination of git blame and git log, you can get that context. git has a feature that can be used with git log where you can search for whether a string/line was added or removed in a range of commits with the -S flag, or if the number of instances of a string/line changed with the -G flag.

> Usually it's not possible to understand a commit without the larger context of the system.

That's true for people who are new to the project, but using something git blame can help people new to the project find the person/people to ask about why that code was written the way it was.

> Generally if some implementation requires an explanation, it can be better understood as a comment next to the implementation in question.

That largely depends on whether people are diligent about updating the comments when they update the code. If you use git blame and look at the associated commit message for that line of code, it will always be up to date. If you make a change to that line of code, then the new commit message would be associated with that line. In fact, a tell tale sign that a code comment may be out-of-date is if the git blame output shows different commits for the comment and the associated line(s) of code.

I notice that some people seem to care more about pretty history than about useful history. I'm not a big fan of rebasing for this very reason: it changes history, which can misinform about what was really done. It can introduce bugs in commits that didn't initially contain them.

Though I will say that git has one big glaring problem with merge commits: they are effectively black boxes, and when you've have a big merge conflict, many git tools don't tell you how that was resolved in that merge. I recently came across a merge that wiped out some of my changes with no clear reason why. I redid that merge and there was no merge conflict at all, so it's still a mystery what happened there.

Still, rebasing has caused far more problems in my experience. It's only suitable for trivial, local cases and never on commits that have already been shared (pushed).

I think a lot of the fear of rebasing comes from not fully grokking what it does, and not rebasing often enough.

My workflow typically involves rebasing my local branches on master every time I switch a branch or start a work session, so typically conflicts that need to be resolved are very clear and small things (e.g. someone else adds a case to a switch statement in the same place your branch does).

The biggest benefit to this that I see is that your diffs are then much more clear in what they're doing, and you have the option to update the commit message to reflect the conflict resolution if required.

That said, if you have a big feature branch and try to rebase it after a month vacation, you're gonna have a bad time.

Daily local rebase on master is really the only acceptable use I know for rebasing. It's not just after a month of vacation, but simply when your local branch has a lot of commits, that rebasing can become very painful, as you may have to resolve the same conflict over and over and over again. That must be the circle of hell where programmers end up.

Basically you'd have to rebase after every single commit. That would keep it the most painless, I guess. But I can also just do a single merge and I'm done.

Git-rerere and smart merge tool like p4merge help a lot with this rebasing problem.

These are essential if you work with forced rebase mode like in Gerrit. (ugh)

You still have to be moderately careful and read the code again.

> Though I will say that git has one big glaring problem

> with merge commits: they are effectively black boxes, and

> when you've have a big merge conflict, many git tools

> don't tell you how that was resolved in that merge. I

> recently came across a merge that wiped out some of my

> changes with no clear reason why. I redid that merge and

> there was no merge conflict at all, so it's still a

> mystery what happened there.

Hmm, I don't think a merge magically removes the need for testing. If I build a feature A that replaces feature B, and another programmer builds a feature C that relies on some aspect of feature B, when merging, although it will merge and compile fine, it's possible it is still broken.

This is a good argument for well defined interfaces, but it's possible to see how a successful merge may not produce a successful source.

As for things disappearing, I think there is some edge case where it looks like one branch deleted something after you made some change to it. Generally it's best to not have people working on exactly the same part of code. On the plus side, you have a full working history, therefore can recover :)

There's more to it than just knowing it's broken. I want to know why and how it happened. Of stuff needs to be tested after a merge; that's how we discovered there was something wrong in the first place. But I don't want to redo the work that I already did; I want the merge to be correct. In this case, it seems the merge did something it should never have done, and it was not clear why this happened.

Still, the commits that were merged both still existed, to it was easy to redo the merge. I shudder to think what would have happened if this had been a rebase; then the original commit might have been gone.

Agree with the thoughts on atomic and meaningful commits (though would drop the merge commit entirely), and I will rebase my commit history extensively at times to get it into a presentable state along the lines listed here.

Bottom line: the history, content, and message of each commit, in addition to the resultant repo state, are all artifacts of your work and are worth shaping and crafting in order to optimize for communicating to yourself and your fellow developers in the future.

> though would drop the merge commit entirely

The merge commit is important though. It helps delimiting a set of commits that have meaning all together from the rest of your commit history. Here's an example:

  a -> b -> c -> d -> e -> f -> g -> h -> i -> j -> k

In the history above, did you notice that commits c through h are actually related to one another? They were introduced a while ago by a feature branch.

Compare the previous history to this:

          c -> d -> e -> f -> g -> h 
  a -> b /                          \ i -> j -> k

Now that's very clear that c through h are part of the same line of work. All those commits were probably part of 1 single pull request and were reviewed in the context of one another.

My history here is still very linear. Yes, there's 1 little branching off, but that's all.

At my previous company we had a codebase of well over 20k commits entirely made of those little branches branching off master, then merging back in. It made it for an incredibly readable history.

An other added advantage of not removing the merge commit is that you can revert the entire merge of your branch in one go.

Without the merge commit, you need to remove each commit one by one (and that's granted you remember which commit was the first one and which one was the last one).

Agreed your history is still linear. But I would suggest your merge commits are perhaps more trouble than they are worth. In commit sequences we prefix almost all commit subjects with the JIRA ticket ID, so the sequenced tickets are quite clearly marked as being related to each other without incurring the merge commit cost.

As for reverting merge commits, I'd rather not personally have to remember the rules as to how to re-apply after the revert (revert the merge revert, then merge the branch again)[1], let alone working on spreading that knowledge losslessly across the organization. This is admittedly something I haven't done much of, but I'd rather revert ~3-9 individual commits, squash the reversion commits (usually with a bit more context on why we're reverting), and push. And it could be scripted. I haven't made a script for it as it doesn't happen that often, but between using rev-parse and a sed script in the GIT_EDITOR environment variable it would be pretty quick.

Having merge commits also means that you have to teach everyone how to view diffs of merge commits, which admittedly isn't that hard, but it is still just one more barrier to know/teach/overcome, and the default behavior of showing a zero-line diff for a no-op merge commit doesn't help.

[1] https://mirrors.edge.kernel.org/pub/software/scm/git/docs/ho...

> Your commit history is what you will use later down the line (think 6 months, a year, 2 years later) to find a very nasty yet incredibly subtle bug in your application.

No, it won’t. If I didn’t catch it then, I will have to read the code and understand it and spot it.

At work, where my builds take anywhere from 30 minutes to 3 hours depending on the target, your git bisect on 1500 commits represents days of building and testing. Or I could just read the code again.

Commit messages should provide context for going through the process of rereading the code, and little else. If a commit message literally describes what the code does (“change constant from X to Y”), it’s self-describing and unnecessary.

It's okay if you really prefer to read a huge squashed commit instead of an atomic commit in a branch, but you made it sound like squashing was necessary in order to reduce building and testing. That's wrong. You can use `git bisect` on merge commits only, then, once the faulty merge is detected, bisect the commits of the merged branch. Bisecting merges won't take more time than bisecting squashed branches.

But can't you just go back and reopen that branch? Just because you squash-merge the branch to master doesn't mean you can't keep the branch archived. I have seen where you basically just "rename" the branch (i.e.: make a new branch of the branch and just leave it as-is) to "merged-2020-03-03-BRANCHNAME" or something and then you don't touch it. Meanwhile, your master is a neat and tidy history of features added, tickets finished, and bugs fixed without all of the "fixed the thing I forgot to add" messages that inevitably sneak their way into your git log.

I think you are being down voted because people are confused about what you are saying.

In git, a "branch" is just a tag on a particular commit. What we intuitively think of as a branch is the set of commits starting from where we diverged on master and ending up with the commit that is tagged with the branch name. There is nothing in git that actually stores that, though. When you rebase the branch, squashing commits, you create a new commit that is that sum of all the other commits. The other commits are left unreachable by any other branch and will be removed the next time you GC the database.

What you are saying (I think) is to add a new branch called "merged-2020-03-03-BRANCHNAME" that points to the same commit as "BRANCHNAME" and then rebase BRANCHNAME, squashing the commits. In that way the old commits are still reachable and won't be removed.

The downside, of course, is that it's a bit of a PITA if you want to find out what commit changed a line of code and why. You would narrow it down to "BRANCHNAME" and then you have to search for the archived version of it, switch to it and then continue looking at your code. But, in principle it could be done.

Yes, that's basically what I'm saying, but I see now it could be a pain with a big repo, so probably not the greatest idea. I do still like a squashed master branch, but I suppose if you trained your developers well enough (ha-ha) they could just commit good messages, however I know several awesome developers who, from time to time, commit a "fixed the thing again" message here and there.

without all of the "fixed the thing I forgot to add" messages

This is why you use rebase --interactive or other ways of amending commits before submitting a PR. It is not an argument for commit squashing.

> rebase --interactive

like 90% of developers (maybe 95%) don't even know how to rebase from master before they submit a PR, so I doubt, highly, that they'll learn how to do a `git rebase -i` in order to make things cleaner. I've personally never seen a repo that utilizes all these "fancy" git commands (fancy to those developers) to make a clean history, I've always seen the messiest shit. Maybe that's sad for me in my career, though.

I agree with you this is the best technical way, though, after reading some more comments in here, but not the best way for everyday developers.

You have made a case for keeping the work-in-progress commits.

The case the other way is that what should matter at the end of the feature development is the visible contribution it makes to the overall project, i.e., the finished code, tests, etc.

This has two fundamental consequences. Firstly, in this model, your final commit that goes onto whatever you call your primary development branch should be a complete, self-contained piece of work, like adding a new feature or fixing some specific piece of broken behaviour. It should include any related tests, data files, etc. It should be something that can be reviewed in isolation and make sense. It should ideally be something that has minimal interactions with any other code that aren't strictly necessary, so it can be removed or replaced later if necessary with minimal effort to resolve or rewrite anything else that has happened since.

Secondly, requiring all work-in-progress commits to also be meaningful and live forever in your shared repo significantly limits the usefulness of source control for the developer during the development stage. They might be experimenting with different approaches but want to keep track of a version where some specific aspect of the code is currently working. They might be about to refactor to clean some new code up but want to make sure they can't lose the exact version they had just got working due to unintended side effects. They don't want to be getting every individual commit code-reviewed or run any slow testing processes, just in case someone later comes along and cherry-picks that particular point in history to use as the basis for something else.

So I will respectfully disagree with you that the full commit history at this level is as important as the final code. My reason for that is that the final code is complete, tested, reviewed, and going into the finished product, while anything in the work-in-progress commits that didn't survive to that point was determined, by the developer doing all of that work, at the time, with the aid of all of the testing and reviews and tools that were involved, not to be as valuable as the final version.

If there really were interesting alternatives considered during development that might be useful to keep around for later reference, you can always tag or branch at that specific point and give it a meaningful name. However, I would suggest that this is like keeping issues open in your bug tracker when you know you'll never fix them: unless you also have some process for keeping track of such ideas and why they are valuable to refer back to them and take some useful action later, you're better off not having them clutter your main development process and tools forever.

I'm actually not arguing the work-in-progess commits should be kept. On the contrary, I do agree that work-in-progress commits are useless, provide no value and should never be submitted for review in a pull request.

I think I should have been more explicit when saying "your commit history is as important as your code". Because I definitely agree that your commit history, representing all the thought process you went through while developing whatever you're coding, does not have its place in the history, and I, as a colleague and reviewer of your code, couldn't care less about it.

I make a distinction between "WIP commits" and "atomic" commits that have well defined boundaries.

While developers work on their feature branches, they should be free of committing however they want, making "WIPs" commits left and right, adding code in one commit, removing that same code in a subsequent one, refactor, type terrible commit messages with no meaning, or not committing at all even.

However, once a feature is ready to be reviewed (and then released), I'm arguing that developers, before submitting their work, should re-organize their commit history: remove WIPs commits, remove refactors that happened during development, squash distinct commits that are part of the same logic, amend commit messages, etc.

The final commit history shouldn't be a series of commits that represent everything that went through the mind of the dev who coded the feature: that's not what I'm arguing for.

The final commit history should simply be multiple small digestible and reviewable chunks of work, that make sense on their own and in the context of the pull request they are part of. Those commits should represent the code in its final, best, most well thought out form.

And once you've gone through the trouble of organizing your work into meaningful, atomic, digestible, chunks of work, with appropriate descriptive commit messages, there's no reason not to merge that work.

As it was useful to the reviewer of your code during the pull-request review, it will also be useful many months later, when you need to review that same old code that seems to be having a bug or to understand why it was written the way it is.

Well, at least, IMO.

> However, once a feature is ready to be reviewed (and then released), I'm arguing that developers, before submitting their work, should re-organize their commit history: remove WIPs commits, remove refactors that happened during development, squash distinct commits that are part of the same logic, amend commit messages, etc.

I agree that this is the optimal solution but isn't this a hefty piece of work that even might introduce subtle bugs?

Eg. I'm thinking of the overall feature needs work to be done in two separate "areas" of the code base and your wip-commits are touching these two areas "separately". Then when you are finished, reordering and squashing the wip-commits into two atomic commits seem like an obvious thing to do. But in a minority of commits you change that tiny little shared part back and forth, from both contexts. Finally you figure out how the shared part should work in the final wip. When you try to reorder the commits the changes in the shared part changes order with leads to merge conflicts when rebasing/reordering/squashing that needs to be solved.

So I'm on the fence of doing that extra work which in practicality could involve an unknown amount of "made up work" to get the clean atomic commits. And those commits then risk being a lie since the reordering, like the shared part in my contrived example, could have made that the first "atomic commit" that never existed also has a bug that never existed that someone down the line potentially stumbles upon when bisecting.

> Finally you figure out how the shared part should work in the final wip. When you try to reorder the commits the changes in the shared part changes order with leads to merge conflicts when rebasing/reordering/squashing that needs to be solved.

An alternative approach is to create a new branch off of master (or whatever the base branch is) and take the diff of the HEAD of your branch with your work in progress commits and stage the hunks of the diff to make a commit that contains a logical change (and repeat the process for the remaining parts of the diff). The git apply command us very useful in this regard.