7 ms·

I prefer Github's method of "git commit messages don't matter, pull requests do". Nowadays, you can easily enforce that the ultimate commit log looks rather ni

I prefer Github's method of "git commit messages don't matter, pull requests do". Nowadays, you can easily enforce that the ultimate commit log looks rather nice by doing this: 1. Make it so the only merge strategy allowed on a repo is "Squash and Merge", so each PR = 1 commit in main branch 2. Have engineers care about the pull request quality rather than commit messages It's easier to be more expressive in a pull request, and intermediate changes while working on a PR aren't super interesting to me.

elpakal 4y ago
If you really want just one commit on your PR you can reset your branch to its target before you merge: git reset --soft <target> Which will undo all commits and leave all modified files in the staging area. Then you can make one commit and force push it to replace your branch @ remote.
- fbormann 4y ago
  Thanks for the tip, man! Really helpful indeed.
- bern4444 4y ago
  You could also rebase and squash all commits into a single one and then rewrite the commit message with git commit --amend.
- xorcist 4y ago
  Or just git rebase -i like normal people.
  - 1ark 4y ago
    Not sure why downvoted, this is the "correct" way. Can also do --autosquash etc. at the same time.
    - elpakal 4y ago
      what makes this more correct than the other way? if anything an interactive rebase requires a lot more ceremony
      - xorcist 4y ago
        Throwing away the commit and doing it again can easily go wrong. It is easy to commit unintended changes by mistake. The interactive rebase is a completely normal operation and intended for exactly this situation. It is also much easier to craft more than one commit, and last minute fixes of spelling errors and such things.
- ninkendo 4y ago
  Careful here… if <target> is not in the ancestry of HEAD, resetting to it may have the effect of undoing any changes that have happened on the <target> branch that aren’t in yours. To be safe, you can do: git reset $(git merge-base <target> HEAD) Which puts HEAD at the last commit in common between your branch and <target>… which, if target is already a parent/ancestor of your commit, is the same thing. But if target has had changed since you branched from it, this prevents you from undoing any recent commits.
mwt 4y ago
This works well enough if (and only if) your company has a culture of small, atomic PRs.
- munk-a 4y ago
  Not really, I recently merged in a two commit branch where one commit was me changing all the vendor configuration for the framework we were using to a new version and the other was all the changes needed to support the change. That PR affected thousands of files in total but the need to frequently rebase the branch to avoid killer merge conflicts encouraged a low commit count. rebase -i can be your friend if you've got a long branch history that adds essentially no value (i.e. "Tried this thing/Didn't work reverting/Tried this other thing/Still no dice/Switching workstations"). An arbitrarily large number of file changes can be packed into a single commit, sometimes for review purposes it makes sense to purposefully isolate different groups of changes in a manner that doesn't mesh with how the dev work was actually done - sometimes I just don't want to have an ugly commit history. I'm allowed to be OCD about my work and sweep the commit where I added print __LINE_NUM__ between each LOC to track down a bug one time that I was too lazy to use gdb under the rug.
  - mwt 4y ago
    Yeah, I mean my comment more as a criticism of universally squash-merging as a policy since, not so much an endorsement of it in general. I run into cases like you describe pretty often, and I doubt I'm alone. Switching to squash-merging has some benefits but it's also brought out a fresh form of hell when too many changes are happening in too many branches at once.
    - munk-a 4y ago
      Oh absolutely - we have a pretty modest sized company and we do tend to "always create merge commit" because it makes some of our deployment tooling easier but otherwise git preferences are left up to the dev and the particulars of the situation.
eikenberry 4y ago
But the PRs aren't part of your git repository? IMO a git repository should be self contained and not require a hosted provide to give context. It lets you manage your work with superior local tooling and without a browser running. Basically I take the exact opposite approach where my PRs are always just a short summary of the commit messages and provides a place for me to put the github specific things like the "Fixes #..". But I make sure my commits are a clean artifact of the changeset by themselves (ie. I do any squashing, etc, on them outside of the PR).
- mwt 4y ago
  GitHub (by default) uses the name of the PR as the merge commit message and also includes the commit message of each commit in the log. Having whitespace-altering "Dummy commit to trigger CI, ugh!" commits in a git history isn't good but it still clutters the `git log` with stock squash+merge GitHub use. I can't speak for everybody, but if GitHub goes down completely and I only had access to my git logs, I'd struggle to recreate ~20% of the information scattered across issues and PRs. This issue is external to merging preferences, but it's definitely not solved by squash-merges and descriptive merge messages.
  - shagie 4y ago
    > Having whitespace-altering "Dummy commit to trigger CI, ugh!" `git commit --allow-empty` may be sufficient for that "there is a new commit" trigger in many cases. If so, that may be preferable to whitespace changes as those clutter up the blame. As an aside, my initial commit on a repo is an empty one so that I can branch from a completely empty repo to do radical rewrites and yet maintain a history relationship with that initial empty commit (which I feel is preferable to an orphan branch and then a merge with unrelated histories ... though those tell slightly different stories in the log).
    - stormbrew 4y ago
      > As an aside, my initial commit on a repo is an empty one so that I can branch from a completely empty repo to do radical rewrites and yet maintain a history relationship with that initial empty commit (which I feel is preferable to an orphan branch and then a merge with unrelated histories ... though those tell slightly different stories in the log). Oh cool, I thought I was literally the only person on the planet to do this lol. I'd do it for branchpoints too except git rebase by default acts very poorly with empty commits in the edited history (deletes them). I wish this was normalized (ie. there was a flag to `git init` to add a commit message for a root commit).
Everlag 4y ago
That's such a different approach than I've grown used to, but different platforms encourage different flows. I've been using bitbucket (not the shiny nice new bitbucket) at work for years and its PR search is so abysmally bad that anything in the PR message but not in the commit message may as well not exist once its merged. `git log` is forever, bitbucket search is /dev/null I'm curious what other affordances or lack of affordances encourage what git behavior.
simonsaysso 4y ago
For the code review step, sure, commit messages don't really matter unless your team reviews PRs commit-by-commit. How many times do you actually change the default squashed message? If you write a series of garbage commit messages, I don't particularly trust that you'll write a very good squashed message, either. How many times do people skip updating the PR description with new information or features from comments? If your commit messages are good, the auto-squash message will be good and one will have a network dependency on GitHub to figure out what decisions went into that change. In general I agree with your goal of a great commit log: 1 PR = 1 commit in the main branch. But I feel like GitHub is just the wrong tool to use if you want that. I used to use Gerrit, where commit messages _are_ your PR description. Sure, it makes you interact with git in some unfortunate ways, but the tradeoff is enforced commit cleanliness.
- woojoo666 4y ago
  Everybody I've seen modifies the squash message in a professional setting. After all, that's the one that everybody else will see. Commits on your feature branch don't really matter and the entire branch can be deleted afterwards anyways
rubyist5eva 4y ago
this works until you want to leave Github for whatever reason
- lelandfe 4y ago
  This happens sometimes when private repos change hands. I've worked at places where we've been given zips of git repos – commit messages reference PR and issue #'s from long distant organizations. I've since started policing PRs whose commit messages reference things inside of our GitHub. At least provide a summary of what you're linking, for the worst case future.
- ericmcer 4y ago
  Why? Wouldnt the commit history contain only the one squashed commit and its legible message?
js2 4y ago
There's a few reasons why this is a lossy workflow compared to commits with proper messages that are later merged, not squashed into one. 1. The PR itself becomes a single giant commit. To adequately explain the diff in a single message would require writing an essay instead of a few paragraphs. It is also now hard to tell which part of the PR message is associated with which part of the massive diff. 2. If something breaks, you can't dissect into which part of the diff caused the problem. 3. It's harder to document your work as you go. I may be working on a PR for days or weeks. Commits give me an opportunity to document each part of my change _at the time I make the change_. It's a chance for me to record how and why at the time it's fresh in my head. If I wait to do this until the PR is ready, I likely will have forgotten some bit of context I wanted to record about the commit. 4. It encourages sloppy commits, making a large PR harder to review. Ideally, I should be able to review a large PR by looking through each commit. If your reply to all this is to say you should just submit smaller and more frequent PRs, fine, but then why ever have PRs that consist of more than one commit?
swalladge 4y ago
> I prefer Github's method of "git commit messages don't matter, pull requests do". This is a bad idea unless it works well for your specific company workflows and you don't care about the future possibility of changing platforms. Git repos are designed to be self contained, decentralised, and offline-first. If you only care about how things look on github, then the repo will have poor usability outside of github - ie. on your workstation, in your local git tools, on a repo mirror, etc. Git commits can be a powerful tool for understanding code if the messages are useful. They are immediately accessible through local tools and can quickly add context to a block of code without breaking immersion. But that immersion is broken as soon as you hit a commit messages like "Merge pull request #123" or "fix bugs".
deleted 4y ago
[deleted]
jen20 4y ago
This would be a lot more reasonable if GitHub would provide tools to review the ultimate commit message in the same manner as the commit content.
Izkata 4y ago
> 1. Make it so the only merge strategy allowed on a repo is "Squash and Merge", so each PR = 1 commit in main branch Please don't do this. It erases history that's really useful for tracking down the cause of bugs. There's plenty of ways to use git log to filter commits if a pretty history is your goal.
woojoo666 4y ago
I agree with this. Oftentimes I am experimenting around (obviously on a separate branch) and use commits as checkpoints. Not every commit is a perfectly polished state. Using separate branches and then squashing when it is ready, gives me the freedom to experiment without constraint, and then clean things up when it is ready
teeray 4y ago
I abhor “Squash and Merge”, because I do take time to make good commits. They each pass tests, they have messages, and I rebase fixup commits away. For me, a merge commit is semantically meaningful (it even gives me a place to put who reviewed it in the merge commit message). This probably seems like a whole lot of work for no benefit. However, I can’t tell you he number of times I’ve been on crappy plane wifi, hit `git blame` on some line and been able to comprehend my state of mind making a change years in the past. Git history is a form of communication.
u801e 4y ago
> I prefer Github's method of "git commit messages don't matter, pull requests do". By doing that, you lock yourself into relying on Github in order to get the context behind a change rather than looking at the commit messages for a particular branch. That means, you cannot easily get that information just using git on the command line. On the other hand, if you put the context in a series of well formatted commit messages, you can get that context by reading through the commit log, either on the command line, or in Github by clicking on each commit. > 1. Make it so the only merge strategy allowed on a repo is "Squash and Merge", so each PR = 1 commit in main branch This leads to very large commits which cannot easily be reverted once any other commits that update the same files are added to the base branch. Also, in this case, the merge commits are essentially redundant, so why have them? There's nothing that prevents one from amending their commit to reference the PR number and eliminate the merge commit entirely. > It's easier to be more expressive in a pull request, and intermediate changes while working on a PR aren't super interesting to me. It really comes down to how those changes are presented. For example, if the change is one commit that adds a new method, and a second commit that adds calls to that method, that makes the change easier to review. Also, if a bug is found in the method, you can make a revert commit to remove all the calls to the new method, another commit to demonstrate the bug with one or more tests, another commit to fix the bug and update the tests to reflect that the fix works as expected. If the commit was just a single PR commit, and another PR was merged, then one would have to craft a commit to undo the changes pertaining to that bug and then make a new commit in another PR to add the updated implementation. This makes it harder to see what the fix was since you essentially remove the entire feature and then commit a new version of that feature.
brunellus 4y ago
The individual commits aren’t interesting until you need them. They should be organized such that they remove the desire to squash them together. Having a well written commit history is in the spirit of the tool and is the more professional approach.

elpakal 4y ago

If you really want just one commit on your PR you can reset your branch to its target before you merge: git reset --soft <target> Which will undo all commits and leave all modified files in the staging area. Then you can make one commit and force push it to replace your branch @ remote.

fbormann 4y ago
Thanks for the tip, man! Really helpful indeed.
bern4444 4y ago
You could also rebase and squash all commits into a single one and then rewrite the commit message with git commit --amend.
xorcist 4y ago
Or just git rebase -i like normal people.
- 1ark 4y ago
  Not sure why downvoted, this is the "correct" way. Can also do --autosquash etc. at the same time.
  - elpakal 4y ago
    what makes this more correct than the other way? if anything an interactive rebase requires a lot more ceremony
    - xorcist 4y ago
      Throwing away the commit and doing it again can easily go wrong. It is easy to commit unintended changes by mistake. The interactive rebase is a completely normal operation and intended for exactly this situation. It is also much easier to craft more than one commit, and last minute fixes of spelling errors and such things.
ninkendo 4y ago
Careful here… if <target> is not in the ancestry of HEAD, resetting to it may have the effect of undoing any changes that have happened on the <target> branch that aren’t in yours. To be safe, you can do: git reset $(git merge-base <target> HEAD) Which puts HEAD at the last commit in common between your branch and <target>… which, if target is already a parent/ancestor of your commit, is the same thing. But if target has had changed since you branched from it, this prevents you from undoing any recent commits.

mwt 4y ago

This works well enough if (and only if) your company has a culture of small, atomic PRs.

munk-a 4y ago
Not really, I recently merged in a two commit branch where one commit was me changing all the vendor configuration for the framework we were using to a new version and the other was all the changes needed to support the change. That PR affected thousands of files in total but the need to frequently rebase the branch to avoid killer merge conflicts encouraged a low commit count. rebase -i can be your friend if you've got a long branch history that adds essentially no value (i.e. "Tried this thing/Didn't work reverting/Tried this other thing/Still no dice/Switching workstations"). An arbitrarily large number of file changes can be packed into a single commit, sometimes for review purposes it makes sense to purposefully isolate different groups of changes in a manner that doesn't mesh with how the dev work was actually done - sometimes I just don't want to have an ugly commit history. I'm allowed to be OCD about my work and sweep the commit where I added print __LINE_NUM__ between each LOC to track down a bug one time that I was too lazy to use gdb under the rug.
- mwt 4y ago
  Yeah, I mean my comment more as a criticism of universally squash-merging as a policy since, not so much an endorsement of it in general. I run into cases like you describe pretty often, and I doubt I'm alone. Switching to squash-merging has some benefits but it's also brought out a fresh form of hell when too many changes are happening in too many branches at once.
  - munk-a 4y ago
    Oh absolutely - we have a pretty modest sized company and we do tend to "always create merge commit" because it makes some of our deployment tooling easier but otherwise git preferences are left up to the dev and the particulars of the situation.

eikenberry 4y ago

But the PRs aren't part of your git repository? IMO a git repository should be self contained and not require a hosted provide to give context. It lets you manage your work with superior local tooling and without a browser running. Basically I take the exact opposite approach where my PRs are always just a short summary of the commit messages and provides a place for me to put the github specific things like the "Fixes #..". But I make sure my commits are a clean artifact of the changeset by themselves (ie. I do any squashing, etc, on them outside of the PR).

mwt 4y ago
GitHub (by default) uses the name of the PR as the merge commit message and also includes the commit message of each commit in the log. Having whitespace-altering "Dummy commit to trigger CI, ugh!" commits in a git history isn't good but it still clutters the `git log` with stock squash+merge GitHub use. I can't speak for everybody, but if GitHub goes down completely and I only had access to my git logs, I'd struggle to recreate ~20% of the information scattered across issues and PRs. This issue is external to merging preferences, but it's definitely not solved by squash-merges and descriptive merge messages.
- shagie 4y ago
  > Having whitespace-altering "Dummy commit to trigger CI, ugh!" `git commit --allow-empty` may be sufficient for that "there is a new commit" trigger in many cases. If so, that may be preferable to whitespace changes as those clutter up the blame. As an aside, my initial commit on a repo is an empty one so that I can branch from a completely empty repo to do radical rewrites and yet maintain a history relationship with that initial empty commit (which I feel is preferable to an orphan branch and then a merge with unrelated histories ... though those tell slightly different stories in the log).
  - stormbrew 4y ago
    > As an aside, my initial commit on a repo is an empty one so that I can branch from a completely empty repo to do radical rewrites and yet maintain a history relationship with that initial empty commit (which I feel is preferable to an orphan branch and then a merge with unrelated histories ... though those tell slightly different stories in the log). Oh cool, I thought I was literally the only person on the planet to do this lol. I'd do it for branchpoints too except git rebase by default acts very poorly with empty commits in the edited history (deletes them). I wish this was normalized (ie. there was a flag to `git init` to add a commit message for a root commit).

Everlag 4y ago

That's such a different approach than I've grown used to, but different platforms encourage different flows. I've been using bitbucket (not the shiny nice new bitbucket) at work for years and its PR search is so abysmally bad that anything in the PR message but not in the commit message may as well not exist once its merged. `git log` is forever, bitbucket search is /dev/null I'm curious what other affordances or lack of affordances encourage what git behavior.

simonsaysso 4y ago

For the code review step, sure, commit messages don't really matter unless your team reviews PRs commit-by-commit. How many times do you actually change the default squashed message? If you write a series of garbage commit messages, I don't particularly trust that you'll write a very good squashed message, either. How many times do people skip updating the PR description with new information or features from comments? If your commit messages are good, the auto-squash message will be good and one will have a network dependency on GitHub to figure out what decisions went into that change. In general I agree with your goal of a great commit log: 1 PR = 1 commit in the main branch. But I feel like GitHub is just the wrong tool to use if you want that. I used to use Gerrit, where commit messages _are_ your PR description. Sure, it makes you interact with git in some unfortunate ways, but the tradeoff is enforced commit cleanliness.

woojoo666 4y ago
Everybody I've seen modifies the squash message in a professional setting. After all, that's the one that everybody else will see. Commits on your feature branch don't really matter and the entire branch can be deleted afterwards anyways

rubyist5eva 4y ago

this works until you want to leave Github for whatever reason

lelandfe 4y ago
This happens sometimes when private repos change hands. I've worked at places where we've been given zips of git repos – commit messages reference PR and issue #'s from long distant organizations. I've since started policing PRs whose commit messages reference things inside of our GitHub. At least provide a summary of what you're linking, for the worst case future.
ericmcer 4y ago
Why? Wouldnt the commit history contain only the one squashed commit and its legible message?

js2 4y ago

There's a few reasons why this is a lossy workflow compared to commits with proper messages that are later merged, not squashed into one. 1. The PR itself becomes a single giant commit. To adequately explain the diff in a single message would require writing an essay instead of a few paragraphs. It is also now hard to tell which part of the PR message is associated with which part of the massive diff. 2. If something breaks, you can't dissect into which part of the diff caused the problem. 3. It's harder to document your work as you go. I may be working on a PR for days or weeks. Commits give me an opportunity to document each part of my change _at the time I make the change_. It's a chance for me to record how and why at the time it's fresh in my head. If I wait to do this until the PR is ready, I likely will have forgotten some bit of context I wanted to record about the commit. 4. It encourages sloppy commits, making a large PR harder to review. Ideally, I should be able to review a large PR by looking through each commit. If your reply to all this is to say you should just submit smaller and more frequent PRs, fine, but then why ever have PRs that consist of more than one commit?

swalladge 4y ago

> I prefer Github's method of "git commit messages don't matter, pull requests do". This is a bad idea unless it works well for your specific company workflows and you don't care about the future possibility of changing platforms. Git repos are designed to be self contained, decentralised, and offline-first. If you only care about how things look on github, then the repo will have poor usability outside of github - ie. on your workstation, in your local git tools, on a repo mirror, etc. Git commits can be a powerful tool for understanding code if the messages are useful. They are immediately accessible through local tools and can quickly add context to a block of code without breaking immersion. But that immersion is broken as soon as you hit a commit messages like "Merge pull request #123" or "fix bugs".

deleted 4y ago

[deleted]

jen20 4y ago

This would be a lot more reasonable if GitHub would provide tools to review the ultimate commit message in the same manner as the commit content.

Izkata 4y ago

> 1. Make it so the only merge strategy allowed on a repo is "Squash and Merge", so each PR = 1 commit in main branch Please don't do this. It erases history that's really useful for tracking down the cause of bugs. There's plenty of ways to use git log to filter commits if a pretty history is your goal.

woojoo666 4y ago

I agree with this. Oftentimes I am experimenting around (obviously on a separate branch) and use commits as checkpoints. Not every commit is a perfectly polished state. Using separate branches and then squashing when it is ready, gives me the freedom to experiment without constraint, and then clean things up when it is ready

teeray 4y ago

I abhor “Squash and Merge”, because I do take time to make good commits. They each pass tests, they have messages, and I rebase fixup commits away. For me, a merge commit is semantically meaningful (it even gives me a place to put who reviewed it in the merge commit message). This probably seems like a whole lot of work for no benefit. However, I can’t tell you he number of times I’ve been on crappy plane wifi, hit `git blame` on some line and been able to comprehend my state of mind making a change years in the past. Git history is a form of communication.

u801e 4y ago

> I prefer Github's method of "git commit messages don't matter, pull requests do". By doing that, you lock yourself into relying on Github in order to get the context behind a change rather than looking at the commit messages for a particular branch. That means, you cannot easily get that information just using git on the command line. On the other hand, if you put the context in a series of well formatted commit messages, you can get that context by reading through the commit log, either on the command line, or in Github by clicking on each commit. > 1. Make it so the only merge strategy allowed on a repo is "Squash and Merge", so each PR = 1 commit in main branch This leads to very large commits which cannot easily be reverted once any other commits that update the same files are added to the base branch. Also, in this case, the merge commits are essentially redundant, so why have them? There's nothing that prevents one from amending their commit to reference the PR number and eliminate the merge commit entirely. > It's easier to be more expressive in a pull request, and intermediate changes while working on a PR aren't super interesting to me. It really comes down to how those changes are presented. For example, if the change is one commit that adds a new method, and a second commit that adds calls to that method, that makes the change easier to review. Also, if a bug is found in the method, you can make a revert commit to remove all the calls to the new method, another commit to demonstrate the bug with one or more tests, another commit to fix the bug and update the tests to reflect that the fix works as expected. If the commit was just a single PR commit, and another PR was merged, then one would have to craft a commit to undo the changes pertaining to that bug and then make a new commit in another PR to add the updated implementation. This makes it harder to see what the fix was since you essentially remove the entire feature and then commit a new version of that feature.

brunellus 4y ago

The individual commits aren’t interesting until you need them. They should be organized such that they remove the desire to squash them together. Having a well written commit history is in the spirit of the tool and is the more professional approach.