Merge-commit noise?


(Jacob Bandes-Storch) #1

Now that contributions are being made on GitHub, it seems there is a lot of
noise in the repo's history from merge commits.

<img src=’/uploads/default/original/1X/2d716bc3c325e040ab3bad753014d3b7f782303f.png’ width=‘215’ height=‘329’>

Would it make sense to instate a policy where commits must be rebased on
top of master before merging/pushing?

(The rebase would have to be done by folks with commit access—namely Apple
employees, for now.)

Just a thought,
Jacob Bandes-Storch


(David Zarzycki) #2

If one uses the command-line to merge branches, rather than GitHub, you can avoid many of these “noisy” merge commits. For background, see: http://ariya.ofilabs.com/2013/09/fast-forward-git-merge.html

···

On Dec 5, 2015, at 02:15, Jacob Bandes-Storch <jtbandes@gmail.com> wrote:

Now that contributions are being made on GitHub, it seems there is a lot of noise in the repo's history from merge commits.

<image.png>

Would it make sense to instate a policy where commits must be rebased on top of master before merging/pushing?

(The rebase would have to be done by folks with commit access—namely Apple employees, for now.)

Just a thought,
Jacob Bandes-Storch
_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


(Jordan Rose) #3

In this case having merge commits is a feature: it records both the original contributor and the Swift team member who accepted it. I know Git allows us to record such information in a single commit, but this doesn't require any extra work.

Another point is that we strive to have every commit on the master branch build and pass all the tests. That doesn't always happen, of course, but the closer we get to that, the better we can do when bisecting the commit history to find when an issue was introduced. Because not everyone submitting a patch has necessarily maintained that rigor (particularly not after rebasing), we use the merge commit to make multiple out-of-tree commits atomic. (But we will send back large pull requests in favor of several smaller ones, per the "Incremental Development <https://swift.org/contributing/#incremental-development>" policy on the website.)

The history may be overly colorful, but it's also a more accurate representation.

Jordan

···

On Dec 5, 2015, at 10:33 , David Zarzycki <zarzycki@icloud.com> wrote:

If one uses the command-line to merge branches, rather than GitHub, you can avoid many of these “noisy” merge commits. For background, see: http://ariya.ofilabs.com/2013/09/fast-forward-git-merge.html

On Dec 5, 2015, at 02:15, Jacob Bandes-Storch <jtbandes@gmail.com> wrote:

Now that contributions are being made on GitHub, it seems there is a lot of noise in the repo's history from merge commits.

<image.png>

Would it make sense to instate a policy where commits must be rebased on top of master before merging/pushing?

(The rebase would have to be done by folks with commit access—namely Apple employees, for now.)

Just a thought,
Jacob Bandes-Storch
_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


(Alex Blewitt) #4

Having a merge commit in this way doesn't mean that you avoid bisect problems in any way. It means you are more likely to find a bad commit.

If you are merging multiple commits there should be at least a build check on each of them instead of only the merge node.

Alex

···

On 5 Dec 2015, at 19:46, Jordan Rose <jordan_rose@apple.com> wrote:

Because not everyone submitting a patch has necessarily maintained that rigor (particularly not after rebasing), we use the merge commit to make multiple out-of-tree commits atomic.


(Jacob Bandes-Storch) #5

Note that recording the original contributor is exactly what happens when
you do `git rebase`: the rebaser will become the Committer, but the Author
remains the same.

···

On Sat, Dec 5, 2015 at 1:25 PM, Alex Blewitt via swift-dev < swift-dev@swift.org> wrote:

> On 5 Dec 2015, at 19:46, Jordan Rose <jordan_rose@apple.com> wrote:
>
> Because not everyone submitting a patch has necessarily maintained that
rigor (particularly not after rebasing), we use the merge commit to make
multiple out-of-tree commits atomic.

Having a merge commit in this way doesn't mean that you avoid bisect
problems in any way. It means you are more likely to find a bad commit.

If you are merging multiple commits there should be at least a build check
on each of them instead of only the merge node.

Alex
_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev