5 ms·

Nirvana is: • Setting `merge.ff=no` in git config to force merge commits by default. • Creating a series of logical commits on `my-feature-branch`. • Merging

Nirvana is: • Setting `merge.ff=no` in git config to force merge commits by default. • Creating a series of logical commits on `my-feature-branch`. • Merging `my-feature-branch` into `main` with a bona fide merge commit. • Using `git branch -d my-feature-branch` (NOT capital `-D`) to delete the feature branch safely and without worry, since `-d` only deletes the branch if the commits are present on HEAD. • Using `git log --oneline --graph` to see a clean representation of the actual history.

pabs3 6y ago
I would only use merge commits when it is appropriate, like a commit series porting usage of a dependency from an old version to a new one.
- rectang 6y ago
  Well, there are different views of "appropriate". When you do team development and everything is done via feature branches, it's nice to have merge commits so that the integrity of the each feature development effort is preserved via a merged branch in the history. If everything is flattened, it's harder to see where the branches (standing in for development initiatives) begin and end. You can't always get fast-forward merges anyway. Long-lived branches with merge conflicts are undesirable but unavoidable in the long run. At least some of the time, you're going to have merge commits even when your "appropriateness" test says there shouldn't be one.
  - pabs3 6y ago
    Do you at least agree that merge commits for single-commit PRs aren't "appropriate"?
    - rectang 6y ago
      I don't feel strongly about the issue. A good clean fast-forward merge of a single-commit PR is fine. But I've also worked at multiple jobs where every merge to the production branch created a merge commit and that's also fine. It adds a bit of complexity to the history graph, but it's not meaningless complexity. If your commit history is majority single-commit PRs then having additional merge-commits everywhere would be noisy, so in that case it would be too much. I don't tend to work on actively developed projects that match that pattern, though. Most feature development involves multiple-commit branches.
    - WorldMaker 6y ago
      Merge commits for single-commit PRs helpfully record which PR # was merged if you need to review/audit the PR sometime later, if nothing else.
      - u801e 6y ago
        The original commit could be amended to include that information.
        WorldMaker 6y ago
        The point of using a "proper" merge commit would be to avoid amending/rebasing the original commit and allow the original commit to live as-developed in the final branch.
        u801e 6y ago
        The only thing changing in the original commit is to include a reference to the PR number in the commit message. There would be no change to the tree referenced by the commit.
        WorldMaker 6y ago
        It's an entirely different commit at that point. If work has already started in another branch based on the original commit (for whatever reason), it can cause merge problems down the road. Again, you are likely going to suggest that you can just rebase this other branch on top of the modified commit, but that's still sweeping possible merge commits under the rug, and again just because that rebase is usually automatic including that the tree references should be the same doesn't mean it is always automatic or doesn't have dangerous repercussions (including training junior devs to rebase often and giving them plenty of ammo for avoidable footguns).
        u801e 6y ago
        I'm talking about a single commit PR in Github or Gitlab. If it's based on the latest version of the base branch, then amending it to include the PR number would allow Github to generate a link to the PR page associated with that commit. That would make the merge commit superfluous at that point. So something like: git commit --amend and editing the commit message. This doesn't introduce any further change to the tree associated with the commit.
masklinn 6y ago
> • Setting `merge.ff=no` in git config to force merge commits by default. I'd rather `merge.ff = only` so git never creates a merge commit from under me. It's a big issue because of `git pull`, that thing should not exist. Most git tools are wholly unable to deal with really merge-heavy graphs, too.
- rectang 6y ago
  A pull is just a fetch followed by a merge. So to solve this problem, just fetch instead of pull! Then do `git merge --ff-only` and if it doesn't work, do the rebase or whatever else to resolve the conflict. I did this long before I set `merge.ff=no`. I hate it when pull creates crappy graphs — it's something I try to help all my colleagues to avoid. I often wish that `git pull` didn't exist.
  - masklinn 6y ago
    > A pull is just a fetch followed by a merge. So to solve this problem, just fetch instead of pull! Of course, that’s what I do. But “git pull” is still a danger, and configuring merge.ff=only protects against that danger.
    - rectang 6y ago
      Why is `git pull` a "danger" if you always use `git fetch`? The configuration setting for merge.ff only affects the local machine. It doesn't generally impact other developers. (Unless you're doing something like setting the system gitconfig on a shared dev box, and setting merge.ff to anything other than the default would be really heavy handed in such an envronment.)

pabs3 6y ago

I would only use merge commits when it is appropriate, like a commit series porting usage of a dependency from an old version to a new one.

rectang 6y ago
Well, there are different views of "appropriate". When you do team development and everything is done via feature branches, it's nice to have merge commits so that the integrity of the each feature development effort is preserved via a merged branch in the history. If everything is flattened, it's harder to see where the branches (standing in for development initiatives) begin and end. You can't always get fast-forward merges anyway. Long-lived branches with merge conflicts are undesirable but unavoidable in the long run. At least some of the time, you're going to have merge commits even when your "appropriateness" test says there shouldn't be one.
- pabs3 6y ago
  Do you at least agree that merge commits for single-commit PRs aren't "appropriate"?
  - rectang 6y ago
    I don't feel strongly about the issue. A good clean fast-forward merge of a single-commit PR is fine. But I've also worked at multiple jobs where every merge to the production branch created a merge commit and that's also fine. It adds a bit of complexity to the history graph, but it's not meaningless complexity. If your commit history is majority single-commit PRs then having additional merge-commits everywhere would be noisy, so in that case it would be too much. I don't tend to work on actively developed projects that match that pattern, though. Most feature development involves multiple-commit branches.
  - WorldMaker 6y ago
    Merge commits for single-commit PRs helpfully record which PR # was merged if you need to review/audit the PR sometime later, if nothing else.
    - u801e 6y ago
      The original commit could be amended to include that information.
      - WorldMaker 6y ago
        The point of using a "proper" merge commit would be to avoid amending/rebasing the original commit and allow the original commit to live as-developed in the final branch.
        u801e 6y ago
        The only thing changing in the original commit is to include a reference to the PR number in the commit message. There would be no change to the tree referenced by the commit.
        WorldMaker 6y ago
        It's an entirely different commit at that point. If work has already started in another branch based on the original commit (for whatever reason), it can cause merge problems down the road. Again, you are likely going to suggest that you can just rebase this other branch on top of the modified commit, but that's still sweeping possible merge commits under the rug, and again just because that rebase is usually automatic including that the tree references should be the same doesn't mean it is always automatic or doesn't have dangerous repercussions (including training junior devs to rebase often and giving them plenty of ammo for avoidable footguns).
        u801e 6y ago
        I'm talking about a single commit PR in Github or Gitlab. If it's based on the latest version of the base branch, then amending it to include the PR number would allow Github to generate a link to the PR page associated with that commit. That would make the merge commit superfluous at that point. So something like: git commit --amend and editing the commit message. This doesn't introduce any further change to the tree associated with the commit.

masklinn 6y ago

> • Setting `merge.ff=no` in git config to force merge commits by default. I'd rather `merge.ff = only` so git never creates a merge commit from under me. It's a big issue because of `git pull`, that thing should not exist. Most git tools are wholly unable to deal with really merge-heavy graphs, too.

rectang 6y ago
A pull is just a fetch followed by a merge. So to solve this problem, just fetch instead of pull! Then do `git merge --ff-only` and if it doesn't work, do the rebase or whatever else to resolve the conflict. I did this long before I set `merge.ff=no`. I hate it when pull creates crappy graphs — it's something I try to help all my colleagues to avoid. I often wish that `git pull` didn't exist.
- masklinn 6y ago
  > A pull is just a fetch followed by a merge. So to solve this problem, just fetch instead of pull! Of course, that’s what I do. But “git pull” is still a danger, and configuring merge.ff=only protects against that danger.
  - rectang 6y ago
    Why is `git pull` a "danger" if you always use `git fetch`? The configuration setting for merge.ff only affects the local machine. It doesn't generally impact other developers. (Unless you're doing something like setting the system gitconfig on a shared dev box, and setting merge.ff to anything other than the default would be really heavy handed in such an envronment.)