So I and emilio were discussing whether or not to squash https://github.com/servo/servo/pull/18750 and it seemed like we have different ideas of how "atomic" commits should be before landing.
When we land pull requests in servo/servo we like individual commits to build so that git bisect works. Do we require them to pass all tests as well? If not, should we? I don't *think* we do, I'm pretty sure we didn't back in 2014, but emilio pointed out we ask for this in CONTRIBUTING.md and I'm confused now In my opinion we should not require tests to pass on individual commits (just the PR), for a variety of reasons: - git blame is used a lot more than git bisect IME . It is very helpful to have as-tiny-as-possible commits for git blame. - I don't think we ever bisect with the entire testsuite ; it's usually just a couple tests - you *can* bisect over first-parent (it's tricky, but there are commands out there you can copy) making this whole thing moot. However when squashing a large pull request you're effectively losing all that information, and that's not something you can work around - This basically forces large refactorings to be a single commit Thoughts? -Manish Goregaokar _______________________________________________ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
