10 ms·

There's an option between #1 and #2 that they're not listing. They could allow merges, but only merges after a rebase on top of upstream produced with --no-ff.

by avar 8y ago

There's an option between #1 and #2 that they're not listing. They could allow merges, but only merges after a rebase on top of upstream produced with --no-ff. The advantage of that is that you get "linear" history as far as the just-pushed topic topic building upon the last one, and you can use merges to group commits, which allows e.g. easy reverting of an entire buggy feature introduced over N commits by reverting out the merge.

desc 8y ago
The rule we use is: DAGgy fixes for preference, then create multiple fix/feature branches and use rebases to eliminate conflicts. The idea is that each individual branch is based on a commit chosen such that it won't conflict when merged. This is generally rather easy to do, and for feature work it's compatible with a 'simple' rebase workflow: simply rebase onto master whenever conflicts start to show up. For bugfixes it's rather useful to be able to base a fix on the commit which caused the bug (or a common ancestor of all maintained releases which have it) and just submit multiple merges. Conflicts are a fact of life: you either deal with them as part of a rebase, or you deal with them when merging. But the testers or reviewers are accepting to merge, and they might not be the right people to resolve the conflicts (plus GitHub's PR workflow provides no nice way to deal with merge conflicts), so the author should fix it. tl;dr: Non-FF non-conflicting merges only, so your changes get integrated as a 'single' commit in first-parent history, but you get to pick where you base the branch. Props for minimising 'copy' branches. Does depend on density and structure of code, of course. If every feature branch is touching the same file, you're going to have problems either way.
gouggoug 8y ago
Yes this. I've enforced this rule with my team for years and our git tree is a completely flat tree with the occasional branch diverging from and merging back into master. Here's what it looks like in practice: https://imgur.com/a/OMplzuv https://imgur.com/a/OMplzuv (although in this example you can see that someone messed up and thought they rebased before merging but actually didn't; mistakes happen.)
- mfontani 8y ago
  > you can see that someone messed up and thought they rebased before merging but actually didn't we use a pre-receive hook which checks that the "oldest ancestor" between the branch being pushed to (i.e. current master's SHA) and the branch being pushed (i.e. the tip of what is to become the new branch) is actually the current master's sha; else, the push is rejected with "Rebase a merged branch before merging it into master". "oldest ancestor" being: * take git rev-list --boundary $current_master_sha..$new_sha * get the last line matching ^- and remove the ^- * that should be $current_master_sha; if not, error.
  - Joky 8y ago
    > we use a pre-receive hook But in the context of this discussion, LLVM is moving to GitHub and there is no possibility of custom pre-receive hook there as far as I know.
    - avar 8y ago
      As discussed in the linked E-Mail thread they're either talking about using GitHub as a publishing platform for something where they do have custom hooks, or making everything go through GitHub pull requests. Using either of those methods they can enforce this. E.g. if they go through PRs just write a script that e.g. grabs the refs for the PR, rebases (and --no-ff merges, depending) the commits there, pushes them to master, and closes the PR as merged. The user running that script would be the only one allowed to commit to "master", and would use its own scripted method of integrating them, instead of clicking "Merge" in the GitLab UI.
      - Joky 8y ago
        LLVM allows direct push to master from its contributor. If we were to go through pull-requests, we wouldn't need anything as GitHub has this setting built-in: https://help.github.com/articles/configuring-commit-rebasing-for-pull-requests/ https://help.github.com/articles/configuring-commit-rebasing...
  - 8y ago
cryptonector 8y ago
So, this doesn't suck. The merge commits are noise, but you can filter it out.

by avar 8y ago

desc 8y ago

The rule we use is: DAGgy fixes for preference, then create multiple fix/feature branches and use rebases to eliminate conflicts. The idea is that each individual branch is based on a commit chosen such that it won't conflict when merged. This is generally rather easy to do, and for feature work it's compatible with a 'simple' rebase workflow: simply rebase onto master whenever conflicts start to show up. For bugfixes it's rather useful to be able to base a fix on the commit which caused the bug (or a common ancestor of all maintained releases which have it) and just submit multiple merges. Conflicts are a fact of life: you either deal with them as part of a rebase, or you deal with them when merging. But the testers or reviewers are accepting to merge, and they might not be the right people to resolve the conflicts (plus GitHub's PR workflow provides no nice way to deal with merge conflicts), so the author should fix it. tl;dr: Non-FF non-conflicting merges only, so your changes get integrated as a 'single' commit in first-parent history, but you get to pick where you base the branch. Props for minimising 'copy' branches. Does depend on density and structure of code, of course. If every feature branch is touching the same file, you're going to have problems either way.

gouggoug 8y ago

Yes this. I've enforced this rule with my team for years and our git tree is a completely flat tree with the occasional branch diverging from and merging back into master. Here's what it looks like in practice: https://imgur.com/a/OMplzuv https://imgur.com/a/OMplzuv (although in this example you can see that someone messed up and thought they rebased before merging but actually didn't; mistakes happen.)

mfontani 8y ago
> you can see that someone messed up and thought they rebased before merging but actually didn't we use a pre-receive hook which checks that the "oldest ancestor" between the branch being pushed to (i.e. current master's SHA) and the branch being pushed (i.e. the tip of what is to become the new branch) is actually the current master's sha; else, the push is rejected with "Rebase a merged branch before merging it into master". "oldest ancestor" being: * take git rev-list --boundary $current_master_sha..$new_sha * get the last line matching ^- and remove the ^- * that should be $current_master_sha; if not, error.
- Joky 8y ago
  > we use a pre-receive hook But in the context of this discussion, LLVM is moving to GitHub and there is no possibility of custom pre-receive hook there as far as I know.
  - avar 8y ago
    As discussed in the linked E-Mail thread they're either talking about using GitHub as a publishing platform for something where they do have custom hooks, or making everything go through GitHub pull requests. Using either of those methods they can enforce this. E.g. if they go through PRs just write a script that e.g. grabs the refs for the PR, rebases (and --no-ff merges, depending) the commits there, pushes them to master, and closes the PR as merged. The user running that script would be the only one allowed to commit to "master", and would use its own scripted method of integrating them, instead of clicking "Merge" in the GitLab UI.
    - Joky 8y ago
      LLVM allows direct push to master from its contributor. If we were to go through pull-requests, we wouldn't need anything as GitHub has this setting built-in: https://help.github.com/articles/configuring-commit-rebasing-for-pull-requests/ https://help.github.com/articles/configuring-commit-rebasing...
- 8y ago

cryptonector 8y ago

So, this doesn't suck. The merge commits are noise, but you can filter it out.