Maintainer & Contributor Guidelines

[jeffbcross]

We should establish some best practices and guidelines for contributors and maintainers of the project. I have some suggestions based on practices that have evolved over the life of the angular project. I'm sure @mattpodwysocki has some ideas based on what has worked with RxJS.

Prior Art:

• AngularJS Contribution Guidelines (For all contributors)
• Angular 2 Triage and Labels Process (For maintainers)
• RxJS Contributing Guidelines

Common Characteristics

• Both claim to adhere to Google JavaScript style guide
• Both emphasize documentation and testing
• Both provide some guidance on how to design/implement changes

Philosophies and Values of Angular Team for Consideration.

These are more focused on work of the maintainers than on community contributors.

• Clean, flat commit history. We never click the green merge button on PRs, but instead we pull down the PR branch and rebase it against master then replace master with the PR branch. See example gist. This reduces noise in the commit history, removing all of the merge commits, and keeps history flat. The flat history is beneficial to tools/scripts that analyze commit ancestry.
• Always green master. Failing master builds tend to cascade into other broken builds, and frustration among other contributors who have rebased against a broken master. Much of our deployment and other infrastructure is based on the assumption that master is always green, nothing should be merged before Travis has confirmed that a PR is green, even for seemingly insignificant changes. Nothing should be merged into a red master, and whomever broke it should drop everything and fix it right away. Fixes should be submitted as a PR and verified as green instead of immediately merging to master.
• No force pushes to master. Only in rare circumstances should a force push to master be made, and other maintainers should be notified beforehand. The most common situation for a justified force push is when a commit has been pushed with an invalid message. The force push should be made as soon as possible to reduce side effects.
• Small, logical commits. A PR should be focused on a single problem, though that problem may be reasonable to be broken into a few logical commits. For example, a global renaming may be best to be broken into a single commit that renames all files, and then a commit that renames symbols within files. This makes the review process simpler easier, so the diff of the meaty commit (where symbols are renamed) can be easily understood than if both were done in the same commit, in which case github would just show a deleted file and an added file.

There's a lot more to our process, but those are more tactical details that can wait until after some consensus is established on core values around contributing.

Additions or alternative perspectives on these values @Blesh and @mattpodwysocki?

[benlesh]

• No changes to performance tests You can add new performances tests, but you can't change the old ones without serious justification. This is because we want to check performance outcomes against both previous code and versions in PRs. If you must change a perf test, it should be done in it's own PR.

[jamiebuilds]

• Always green master
• No force pushes to master

You might want to use GitHub's protected branches to enforce these.

[benlesh]

@thejameskyle good tip. I didn't know about that feature. master is now protected from force push.

[benlesh]

Okay, I've got a start in, mostly copy pasta from this issue with a tip of the hat to @jeffbcross embedded for fun.

a3f3746

Closing.