Comment by jasode | Hacker News Reader

Comment by jasode | Hacker News Reader

Parent Full thread

>Seeing how the sausage was actually made, ... it is meaningful and will tell you a great deal about a project and its developers... I trust that.
...tidy commits are aberrations and full of little lies...

...small, harmless lies.

Interesting choice of words.

Here's another way to think about squashing private commits for public consumption: programmers do not install keyloggers and upload their entire keystroke history including every Backspace and Ctrl+Z used in their text editor to the repositories. And, most of us wouldn't care about seeing it.

Whether John typed "x = 218^H^H73" or "x = 273" is a meaningless distinction and irrelevant noise. Those spurious ^H Backspaces are equivalent to the twitchy multiple commits in private branches. We really don't want to see them. Think of private noisy commits as an extended workspace of a text editor. If squashing those commits is a lie, the Backspace key without an audited keystroke log is also a lie.

Side note: The other comment downthread about keeping all private commit history for "git bisect" is a red herring. Sometimes a commit will deliberately have broken syntax -- e.g. make a quick commit before getting up to grab a soda -- it won't be CI test worthy. Besides, an automated CI server's cpu cycles can point to an upstream integration/test/qa branch instead of a programmer's private branch.

«If squashing those commits is a lie, the Backspace key without an audited keystroke log is also a lie.»

In a world with infinite storage space and a good UX on top of it, I could absolutely see a case where it might be amazing to have a source control integration with the full undo stack of my editors. VCR roll through someone's efforts Twitch style and grab a box of popcorn as you drinking game your way through their typos...

That said, I definitely will rebase/squash local WIP stuff on local-only branches on my own machine, as I see fit. Yeah, I see those as harmless lies because I really didn't build it that way, but sometimes that's what makes me feel better about publishing that work.

I appreciate you trying to push this conversation towards it's extreme, absurd ends, but I also realize that there are a lot of aesthetic judgments here and I for one lean towards keeping more of the little pieces and the interesting digressions like here's where I totally "brb grabbing a soda" the whole branch and sometimes you taking that break means easier commits for me to review when I'm reviewing your code (whether a code review in a PR immediately, or a research effort down the line) as maybe I need a review break there too. I appreciate not everyone feels the same on this topic.

>you trying to push this conversation towards it's extreme, absurd ends,

I didn't think of my example as absurd hyperbole. People actually do use "git commit" on their local unpublished branch as another form of Backspace/Ctrl+Z/Ctrl+S. And just like every text-editor Ctrl+S keystroke is not meaningful, every "git commit" is not meaningful either. A lot of commits are just the programmer's personal unhygienic work-in-progress scratchpad stuff. It's not CI or "git bisect" worthy.

>I for one lean towards keeping more of the little pieces and the interesting digressions like here's where I totally "brb grabbing a soda" the whole branch

I won't dispute that you like whatever you like (VCR-playback of every keystroke mistake) because we're all different. However, I'm pretty sure most busy programmers reviewing pull requests will not appreciate having to wade through all the commits about "cleaned up whitespace" and "savepoint before soda run."

I think it is hyperbole given the surrounding context of this article/thread is mostly speaking to squashing commits after a review has happened in a PR. I say that not as a judgment, I like hyperbole and admit my top-most comment was intentionally hyberbolic too. Hyberbole is a good conversation to have sometimes.

A commit to git is a named snapshot of a file tree. That's it. All the other "worthiness" we ascribe to a commit is communal. A person's unhygienic work-in-progress can give you insight into their thought process or their work ethic. Why did this person give me this particular named snapshot of their file tree? I expect they are tell me (or future them) something and I can respect that artifact and preserve it, just as easily as you might angrily suffer from it and destroy it.

CI or "git bisect" "worthy" are orthogonal considerations and also vary between communities and needs.

If there is something we can agree on here: yes, all commits are not created equal. Even all commits aren't worth preserving equally. That said, I think there are tooling solutions here far beyond "destroy that what you dislike".

>Why did this person give me this ...

Just to be clear, your premise of "give me" is flawed because not every "commit" was meant as a capital "C" Commit-a-Logical-Unit-of-Work. Instead, many commits in private local history are a more mundane commit-as-a-backup-checkpoint-savepoint. The "savepoints" are like "^H Backspace". Asking "why the programmer gave you those particular savepoints" is like asking "why did he gave you those ^H Backspace keystrokes". It's a nonsensical question.

The confusion is that the same "git commit" command is used for 2 very different semantic purposes:

(1) git commit -m "fixed bug #23984" --> as Logical-Unit-Work

(2) git commit -m "wip" --> as meaningless backup/savepoint like Ctrl+S save

The type (2) was for the programmer's internal purposes of safety backups, cleaning up whitespace, typos in comments, reflexive muscle memory of saving often, etc. They have no semantic meaning to "give you". Type (2) commits can have deliberate broken syntax and they're not meant to be built or be bisected.

Type (2) commits should never be discouraged because saving work often (including broken midstream work) is a good habit but from an outsiders perspective of the reviewers upstream, they are way too noisy. The spurious commits could be less than 30 seconds apart with no compile/build step in between.

The only issue is that most people reviewing the commit history will ascribe a value to the worthiness if a commit.

Linus Torvalds has described his thoughts on rebasing:

http://torvalds-family.blogspot.com.au/2009/06/happiness-is-...

It's not destroyed, it's simply not on master. If you want to push that branch and have the history forever, that is an option.

Anything not stored on the system of record cannot be counted on to exist at any point in the future.

From a strategic standpoint, thinking of that data as anything other than 'destroyed' would be a grave error of judgement. From a retention standpoint the difference between 'could' and 'did' is profound.

> Anything not stored on the system of record cannot be counted on to exist at any point in the future.

That's an issue with GitHub itself, not an issue with the method of squashing commits before merging into master. See gerrit as a prime example of separating reviewing code and your master branch.

If you have an issue with people deleting branches that were intentionally preserved to save history that was then squashed onto master, that's not the fault of anyone who squashed their commits but left them preserved on a branch.

As a code reviewer looking at a pull request, why aren't you just looking at the entire diff instead of individual commits? How is squashing to a single commit and looking at that different than just viewing the entire diff for lots of little commits?

> you trying to push this conversation towards it's extreme, absurd ends

I would remind you that you are the one introducing morally charged terms like "lying" when describing rebasing. The fact that you cannot conceded even that those who disagree with you are at least morally good actors is the source of the extremism in this conversation.

I would encourage you to seek hard to understand how it could be that morally good people still conclude that rewriting project commit history is a desirable thing, rather than concluding that they're engaged in self/external deception. At the very least, your conversations about the topic will become more productive.

It's inherently deceptive. Instead of thinking through problems to produce their solutions in an ordered, clear fashion, they make a big spaghetti mess, and then hide the sausage-making in a squashed commit that throws away work that may in fact, be very useful later on, all to present the illusion that they arrived at the visible conclusion without misstep.

That's deceptive, wasteful, and has more to do with ego or shallow ideas about "cleanliness" than it does with usefully recording our work processes.

The kind of work you're thinking of is generally what you put in comments.

Someone reading the code will not be reading the commit log. If there's gotchas in the code, they should be expressed in comments, not in the commit log.

The commit log, on the other hand, should always be readable for devs, especially newcomers to the project, to get an idea of how to develop small and concise features and how to contribute. Smaller, cleaner and always-buildable commits also make for very easy git bisects.

Git isn't here to record your personal history with the code, it's here to provide developers with an understanding of how the project evolved. It's not deceptive to arrange your commits to make them clean, it's part of creating a clear, understandable, easily-reviewed unit of change.

What you're missing is that "gotchas in the code" are not always known when the code is being written.

Very often, you have to piece together what happened and why from the record of what people actually did.

There's no such thing as "personal history" with the code in the context of SCM commits. It's all project history.

This new idea is both deceptive, and honestly, shocking. No SCM system previously has ever been used to actively encourage discarding history, and decades of using these SCM systems ought to have adequately demonstrated why that history matters.

>There's no such thing as "personal history" with the code in the context of SCM commits. It's all project history.

Yes, there absolutely is "personal history" with DVCS.

Your rigid mental model for "SCM commits" is relevant for older tools like cvs/svn with lock-checkout-checkin-unlock procedures. The "commits" in those centralized SCMs are a Really Big Deal.

DVCS is different. When a programmer decides to "fork" a public repository, he copies it to "local" harddrive and it becomes a personal repository. From that fork, the programmer can also create an unpublished branch and that branch is another level of personal scratch space. The commits in this type of environment are not a big deal. With git/dvcs, it shifts the Important Action from the "commit" to the "merge/fast-forward" step. That's why many programmers rebase to optimize how that step looks.

The concept of "personal history" is built-in into the ethos of how Git and DVCS works and it's been a separate concept from "public project history" since day one. The programmer can certainly choose to share every untouched line of personal history but he doesn't have to. Same concept as not sharing a recording of memory buffers and a log of every keystroke from the text editor. Most of us don't care to see any of that.

No, programming process is not different.

I'm very aware of what DVCS is. It doesn't require breaking our collaborative processes just because changes are staged locally first.

>our collaborative processes just because changes are staged locally first.

The keystrokes of ^H Backspace are not a "collaborative process". The idiosyncratic persistence of RAM buffers of a text editor is also not a "collaborative process". Why do you think "git commit" on a local unpublished repo is collaborative? I can only guess that it's because the word "commit" is in "git commit"? Well, syntax is not semantics.

>I'm very aware of what DVCS is.

I think many people with your position know the commands syntax of DVCS but don't actually grok the "draft work" philosophy of DVCS. Therefore you assign label of "collaborative process" to unrelated concepts such as the typing habits the programmer happens to execute on his laptop's harddrive. Unfortunately, the programmer persisting his editor's RAM buffers to disk happens to use the command "git commit" which distorts the thinking and causes people to label that action as "collaborative"!

The "pull request" and "merge" is collaborative. The raw "git commit" is not.

Not all lying makes you a bad person (thank you so much for the gift! I love it!).

But it's still lying.

Except it's not lying. If the unit of work is good, then watching the developer go backwards and forwards and making mistakes that stop the software from even compiling isn't even helpful - in fact, you can sometimes get multiple of these errors stack up in a row. Trying to bisect, even with bisect skip, is painful in these cases, and it doesn't even help with code reading.

The general rules of Linus Torvalds are the only real sane ones in this situation, at least IMO:

1. For a clean commit, make sure you rebase only on your private history. If the branch is public and in any way used by anyone else, then unfortunately it's not something that can be considered "private" any more (at least not unless you give strict warnings that no serious work should be done on the code in the branch until it is finalized).

2. Once your code is committed into a public branch or into the master branch, then you can't change it. No rebasing on this code!

3. Do NOT rebase other people's code, even if it is very messy. If you pull in messy code, then complain about their messy code or try to avoid depending on it. Which basically means that developers that collaborate need to practice pushing and merging clean code.

I said elsewhere that the only use of squash that I strongly agree with is to remove bugs and other dumb mistakes.

What I disagree with strenuously, to the point of anger, is having a code as where every line of blame tells me only who made a change and what feature it was for. And I know too many people who think of every feature or pull request as a single commit.

THAT is lying, and I mean that in the judgement-passing definition of the word. You're fucking up the version history intentionally, and you should be stopped.

I'm not in the habit of reading code for fun. I have a job to do, and I want to do it. If I'm looking at the version history it's pretty much only for two reasons. Maybe I know for a fact there was a clever bit of code in this file and it's gone now, so I need to find it. Was it deleted or did you split isn't off into another file? I like those but they don't happen to me very often.

More often I'm reading blame because of a wtf moment. I can't think of a logical reason why the code looks the way it does, but I need to make a change and so I need to know if the person who wrote it was trying to do accomplish something or just confused.

Commit a8bcdef by dingus Implement feature #532

That tells me what changed, who changed it, how it changed and when, but I've lost the why. I know, you probably think "feature 532 is why, duh" but it doesn't tell the story of the intermediate states and why for instance they changed 'users > 1' to 'users >= 1'.

And if there is one class of commit should absolutely and under no circumstances ever ever be squashed it's any commit where you reformatted code. If a line changes and its part of a commit for a feature I'm going to assume that method body was all you, and stop looking. If you 'wrote' it but the commit message is "pretty print" I know I have to go pull a blame on the parent version to keep digging.

Ah, yeah - I agree with that last bit about formatting changes - even if I have probably unintentionally violated that in the past.

However, if you have changed users > 1 to users >= 1 and there is some sort of intrisic logic to it, surely a. you put that reasoning into the commit message, and b. you add a comment into the code briefly explaining the significance?

I don't know about you, but I'd prefer to know about why this is significant within the code, rather than have to dig through the blame logs of a version control system!

Early versions of Microsoft Word kept the undo history in the .doc file. I remember reading about a few cases where someone wrote things like "my boss is an ass", erases it and then sends the report to the boss, boss accidentally clicks undo too much and guess what he sees.

Reply on news.ycombinator.com