4 ms·

> Third, Paragon's repository has commit messages which lack information, like "Merge branch 'torvalds:master' into master." > Torvalds said that "Linux kernel

> Third, Paragon's repository has commit messages which lack information, like "Merge branch 'torvalds:master' into master." > Torvalds said that "Linux kernel merges need to be done properly." He added: "That means proper commit messages with information about what is being merged and why you merge something. But it also means proper authorship and committer information etc. All of which github entirely screws up." 100% agreed with this. On a related note I truly despair a little when people insist on blanket "squash everything" workflows where the source control UI defaults to (or has been configured to only allow) squash merges. It goes from one extreme of "removing wip and todo commits from the history" to "remove commits almost entirely and lose all the context". I'm guilty of the first one, but I could locally squash to remove the messier commits but instead the nuclear option is picked.

noptd 5y ago
When squash merging, all commit messages are added to the extended description in the editor, which makes it far easier to remove useless comments and add in any necessary info as needed. All of that context can then be viewed via git show and other commands, while keeping the base branch's history linear and readable in the process. I don't see a problem with squash merging, especially as a point of policy. It's been the best of both worlds in my experience.
- throwawaylinux 5y ago
  Which are the two worlds it is the best of? Not the regular merge, because the best of that world is that you have individual commits that you can reason about and bisect. Unless your branch history is full of junk, unfinished commits, etc. But in that case the policy should be to not publish such branches (or at least only publish them as WIP / scratch branches which will get cleaned up properly and rebased before upstream merge).
- u801e 5y ago
  But they also lead to commits that affect a lot of lines and files and are much harder to revert without conflicts.
im-poor 5y ago
What value do those commits really provide? In master, I want every commit to represent a change in state that I may want to revert to when rolling back or bisecting. I don’t ever want to go back in time mid-way through someone’s feature development. What use is that? If I’m curious about the specific history of that feature, I’ll go to the branch it came from.
- smcameron 5y ago
  > mid-way through someone’s feature development. If someone is doing it right, any point in their "mid way" that they expose will be git bisectable. Otherwise, they're doing it wrong. Squashing 20 already decent git bisectable commits into 1 big one is the complaint here. Squashing 20 shit commits into a giant ball of shit that happens to compile and run is not better though. Don't make the 20 shit commits in the first place.
- u801e 5y ago
  If commits are split properly, then it's not an issue. For example, one commit could add a new method, and a subsequent commit could add one or more calls to that method. If it turns out that we don't want to use the new method without some modifications, then the commit that adds calls to that method could be reverted insured of having to revert the entire implementation.
  - ryan_lane 5y ago
    It's a lot easier to control what goes into the main branch than what goes into someone's feature branch. Who's going to police the feature branches to ensure the merges are cleaned up and rebased prior to merge to main? My feature branches are full of useless commits (from the perspective of the main branch), and anyone that tells me to clean that up prior to a merge can pound dirt. Of course, my repos use squash merge, so I'd never get that complaint.
    - u801e 5y ago
      > Who's going to police the feature branches to ensure the merges are cleaned up and rebased prior to merge to main? That's typically handled when the code is reviewed. > My feature branches are full of useless commits (from the perspective of the main branch), and anyone that tells me to clean that up prior to a merge can pound dirt. If I were reviewing it, then you would not get my approval until the commit history is cleaned up. It would be the same result if your code was failing a test scenario, not making it past the linter, or if someone asked you to clarify what's being done by adding a comment block.
MathMonkeyMan 5y ago
Solution: Make Github's PR merge and/or squash commit message the PR extended description exactly, prefixed with a permanent link to the PR. Then the trick is making your thoughtful well-edited commit messages into a thoughtful well-edited PR description. Problem then is that the initial PR description does not often reflect review discussion. I think it's better to encourage requestors to edit their description than to believe that the person hitting the big "you're done" button will sit and edit a commit message.

noptd 5y ago

When squash merging, all commit messages are added to the extended description in the editor, which makes it far easier to remove useless comments and add in any necessary info as needed. All of that context can then be viewed via git show and other commands, while keeping the base branch's history linear and readable in the process. I don't see a problem with squash merging, especially as a point of policy. It's been the best of both worlds in my experience.

throwawaylinux 5y ago
Which are the two worlds it is the best of? Not the regular merge, because the best of that world is that you have individual commits that you can reason about and bisect. Unless your branch history is full of junk, unfinished commits, etc. But in that case the policy should be to not publish such branches (or at least only publish them as WIP / scratch branches which will get cleaned up properly and rebased before upstream merge).
u801e 5y ago
But they also lead to commits that affect a lot of lines and files and are much harder to revert without conflicts.

im-poor 5y ago

What value do those commits really provide? In master, I want every commit to represent a change in state that I may want to revert to when rolling back or bisecting. I don’t ever want to go back in time mid-way through someone’s feature development. What use is that? If I’m curious about the specific history of that feature, I’ll go to the branch it came from.

smcameron 5y ago
> mid-way through someone’s feature development. If someone is doing it right, any point in their "mid way" that they expose will be git bisectable. Otherwise, they're doing it wrong. Squashing 20 already decent git bisectable commits into 1 big one is the complaint here. Squashing 20 shit commits into a giant ball of shit that happens to compile and run is not better though. Don't make the 20 shit commits in the first place.
u801e 5y ago
If commits are split properly, then it's not an issue. For example, one commit could add a new method, and a subsequent commit could add one or more calls to that method. If it turns out that we don't want to use the new method without some modifications, then the commit that adds calls to that method could be reverted insured of having to revert the entire implementation.
- ryan_lane 5y ago
  It's a lot easier to control what goes into the main branch than what goes into someone's feature branch. Who's going to police the feature branches to ensure the merges are cleaned up and rebased prior to merge to main? My feature branches are full of useless commits (from the perspective of the main branch), and anyone that tells me to clean that up prior to a merge can pound dirt. Of course, my repos use squash merge, so I'd never get that complaint.
  - u801e 5y ago
    > Who's going to police the feature branches to ensure the merges are cleaned up and rebased prior to merge to main? That's typically handled when the code is reviewed. > My feature branches are full of useless commits (from the perspective of the main branch), and anyone that tells me to clean that up prior to a merge can pound dirt. If I were reviewing it, then you would not get my approval until the commit history is cleaned up. It would be the same result if your code was failing a test scenario, not making it past the linter, or if someone asked you to clarify what's being done by adding a comment block.

MathMonkeyMan 5y ago

Solution: Make Github's PR merge and/or squash commit message the PR extended description exactly, prefixed with a permanent link to the PR. Then the trick is making your thoughtful well-edited commit messages into a thoughtful well-edited PR description. Problem then is that the initial PR description does not often reflect review discussion. I think it's better to encourage requestors to edit their description than to believe that the person hitting the big "you're done" button will sit and edit a commit message.