undefined | Better HN

0 pointschris_wot10y ago0 comments

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.

0 comments

2 comments · 1 top-level

hinkley10y ago· 1 in thread

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.

chris_wotOP10y ago

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!

j / k navigate · click thread line to collapse