5 ms·

The biggest problem with commit messages: They lie. The main reason for this is that most people tend put more than one atomic change into a single commit, but

The biggest problem with commit messages: They lie. The main reason for this is that most people tend put more than one atomic change into a single commit, but in the message they often only mention the "main change" which they introduced with the commit. This in turn makes it much harder to find out which commit introduced a given problem into the code base since people will usually read the (incomplete) message of a given commit and think "the problem can't be here because this commit only did X" while it actually did Y & Z as well. So, personally, I think we could as well abandon writing commit messages entirely and instead make sure that individual changes are small enough so that we can figure out what happened by looking at the actual code instead. As an example, if you look at individual commits in the Django project (such as this one: https://github.com/django/django/commit/e7e8d30cae9457339eb49ae8584e82ff0a038e99 https://github.com/django/django/commit/e7e8d30cae9457339eb4...), it is often much easier to figure out what happened by looking at the file diff than the commit message. A tool that could summarize changes in source code using advanced (semantic) diffing instead of line-by-line diffs would make this much easier of course.

seiji 11y ago
most people tend put more than one atomic change into a single commit, That's a people problem. Correct it through proper training and mentoring, not letting it continue just because "that's what people do." Historically, some people were afraid of "wasting" commit numbers (CVS, SVN, mock revision numbers in hg), but git has no concept of an incremental commit number, so you can burn through as many commits as you want without feeling guilting about running up an auto-incrementing counter.
- xorcist 11y ago
  How do you "waste" commit numbers? They're just numbers. I believe the concern is about log space. (And git has a log too.) Hopefully someone reads that log and they shouldn't be bothered by a thousand trivial changes, the reasoning goes. Which is true to some extent, it's just that everyone doesn't get it right ... and that's where your comment about mentoring and training comes in. It must also be ok to make mistakes as to not try to hide slip-ups in the next commit.
  - donatj 11y ago
    I have worked with people who are obsessive about keeping auto increment numbers in databases "tidy". It's obviously nonsense but some people aren't logical. The same thing applies to some projects fears of actually following semantic versioning. Numbers are infinitesimally cheap, there should be no fear about burning them.
danieltillett 11y ago
You don’t need a magic tool to do this - what we need is a way to block commits that don’t have corresponding documentation of the changes. A tool that made you write a comment message for each code change block would get the job done - if you commit 10 changes then you would have to write 10 messages to explaining each of the changes.
deleted 11y ago
[deleted]
deleted 11y ago
[deleted]
jammycakes 11y ago
If you're using pull requests and reviewing them before they're merged, you could make inaccurate or incomplete commit summaries grounds for rejection of the request. Ask them to fix it using interactive rebase.
- gkop 11y ago
  I'm curious how many of us interactive rebase every patchset before merging? For me it's critical because I tend to commit too frequently.
  - EmanueleAina 11y ago
    For what is worth, microcommits + rebase for fast-forward merges are the basis of the workflow used in the GNOME project. As far as I know, it's more or less also what it is used by kernel people before hitting the tree of a maintainer (and it's non-ff merges from there).
jumpwah 11y ago
With magit, it's easy to only stage individual 'hunks' so that you can have only the relevant parts of a file staged. And so this problem goes away. I also imagine it's easy to do this in fugitive as well. It's probably also possible on the cli, but I imagine that would be too 'hard' or time consuming, hence you will probably start to be lazy about doing it correctly.
- ckuehl 11y ago
  You can simply use `git add -p` to stage hunks individually (or `git reset -p` to unstage some hunks).
  - deleted 11y ago
    [deleted]
  - aninhumer 11y ago
    And `git checkout -p` to erase hunks from the working directory. (i.e. delete without saving!)
    - icebraining 11y ago
      That's a little dangerous! I use git stash -p instead, and only drop it after I'm absolutely sure.
  - 0x0 11y ago
    "git add -p" can be fun, but it's a little scary because it's super easy to end up making commits that don't actually stand on their own. Super simple to miss an import here or a new field there. If you do a whole series of them, it might actually be worse for others to come back to (or bisect in) if they don't realize the original developer never actually compiled and tested each commit as-is.
    - aninhumer 11y ago
      Well ideally you could use `git stash -k -u` (-keep index, stash -untracked) to set your working directory to the state you're actually committing, and then run some tests. For the most part though, I just try to avoid having so many hunks to step through that this is even an issue.
caf 11y ago
I would think that scanning through commits is a pretty inefficient way to find out which commit introduced a problem anyway. "git bisect", "git blame" and "git log -S" are my tools of choice.
aikah 11y ago
> more than one atomic change into a single commit There is no such thing as an "atomic change". Sometimes, fixing a single bug ,adding a single feature requires the edition of multiple files or even complex changes. I personally don't like these projects with 1 commit per file change ,that's ridiculous and it's noisy.
- gutnor 11y ago
  I think he mean a single feature. Like a single commit "Fix bug XYZ", that in reality also contains "Fix typo in error messages", "Change rendering of status page", "Fix test framework DSL to prevent infinite loop". Naively you could say that each of those should go in their own commit, but reality is that they may actually be quite small and necessary and may not even be seen as a feature by the author or reviewer. Only 6 months down the line, you read the code and wonder why the f*ck there is a change in the test DSL in order to fix bug XYZ.
- nanny 11y ago
  >Sometimes, fixing a single bug ,adding a single feature requires the edition of multiple files Congrats, that's one atomic change. He didn't say "one commit per file change", he said "one commit per atomic change", and yes, sometimes those atomic changes can spread across multiple files.

seiji 11y ago

most people tend put more than one atomic change into a single commit, That's a people problem. Correct it through proper training and mentoring, not letting it continue just because "that's what people do." Historically, some people were afraid of "wasting" commit numbers (CVS, SVN, mock revision numbers in hg), but git has no concept of an incremental commit number, so you can burn through as many commits as you want without feeling guilting about running up an auto-incrementing counter.

xorcist 11y ago
How do you "waste" commit numbers? They're just numbers. I believe the concern is about log space. (And git has a log too.) Hopefully someone reads that log and they shouldn't be bothered by a thousand trivial changes, the reasoning goes. Which is true to some extent, it's just that everyone doesn't get it right ... and that's where your comment about mentoring and training comes in. It must also be ok to make mistakes as to not try to hide slip-ups in the next commit.
- donatj 11y ago
  I have worked with people who are obsessive about keeping auto increment numbers in databases "tidy". It's obviously nonsense but some people aren't logical. The same thing applies to some projects fears of actually following semantic versioning. Numbers are infinitesimally cheap, there should be no fear about burning them.

danieltillett 11y ago

You don’t need a magic tool to do this - what we need is a way to block commits that don’t have corresponding documentation of the changes. A tool that made you write a comment message for each code change block would get the job done - if you commit 10 changes then you would have to write 10 messages to explaining each of the changes.

deleted 11y ago

[deleted]

deleted 11y ago

[deleted]

jammycakes 11y ago

If you're using pull requests and reviewing them before they're merged, you could make inaccurate or incomplete commit summaries grounds for rejection of the request. Ask them to fix it using interactive rebase.

gkop 11y ago
I'm curious how many of us interactive rebase every patchset before merging? For me it's critical because I tend to commit too frequently.
- EmanueleAina 11y ago
  For what is worth, microcommits + rebase for fast-forward merges are the basis of the workflow used in the GNOME project. As far as I know, it's more or less also what it is used by kernel people before hitting the tree of a maintainer (and it's non-ff merges from there).

jumpwah 11y ago

With magit, it's easy to only stage individual 'hunks' so that you can have only the relevant parts of a file staged. And so this problem goes away. I also imagine it's easy to do this in fugitive as well. It's probably also possible on the cli, but I imagine that would be too 'hard' or time consuming, hence you will probably start to be lazy about doing it correctly.

ckuehl 11y ago
You can simply use `git add -p` to stage hunks individually (or `git reset -p` to unstage some hunks).
- deleted 11y ago
  [deleted]
- aninhumer 11y ago
  And `git checkout -p` to erase hunks from the working directory. (i.e. delete without saving!)
  - icebraining 11y ago
    That's a little dangerous! I use git stash -p instead, and only drop it after I'm absolutely sure.
- 0x0 11y ago
  "git add -p" can be fun, but it's a little scary because it's super easy to end up making commits that don't actually stand on their own. Super simple to miss an import here or a new field there. If you do a whole series of them, it might actually be worse for others to come back to (or bisect in) if they don't realize the original developer never actually compiled and tested each commit as-is.
  - aninhumer 11y ago
    Well ideally you could use `git stash -k -u` (-keep index, stash -untracked) to set your working directory to the state you're actually committing, and then run some tests. For the most part though, I just try to avoid having so many hunks to step through that this is even an issue.

caf 11y ago

I would think that scanning through commits is a pretty inefficient way to find out which commit introduced a problem anyway. "git bisect", "git blame" and "git log -S" are my tools of choice.

aikah 11y ago

> more than one atomic change into a single commit There is no such thing as an "atomic change". Sometimes, fixing a single bug ,adding a single feature requires the edition of multiple files or even complex changes. I personally don't like these projects with 1 commit per file change ,that's ridiculous and it's noisy.

gutnor 11y ago
I think he mean a single feature. Like a single commit "Fix bug XYZ", that in reality also contains "Fix typo in error messages", "Change rendering of status page", "Fix test framework DSL to prevent infinite loop". Naively you could say that each of those should go in their own commit, but reality is that they may actually be quite small and necessary and may not even be seen as a feature by the author or reviewer. Only 6 months down the line, you read the code and wonder why the f*ck there is a change in the test DSL in order to fix bug XYZ.
nanny 11y ago
>Sometimes, fixing a single bug ,adding a single feature requires the edition of multiple files Congrats, that's one atomic change. He didn't say "one commit per file change", he said "one commit per atomic change", and yes, sometimes those atomic changes can spread across multiple files.