Diagrams/Dev/ReviewingAndMerging

So, you have push access to one or more of the repos in the diagrams organization on github. Now what?

• Do code reviews. When someone opens a pull request for a repository to which you have push access, you are welcome to review their code. (You're also welcome and encouraged to review pull requests on other repositories as well.) At a minimum this means reading carefully through their proposed changes and commenting; ideally it also means downloading and testing their code.

  Some guidelines for code reviews:

  • Be constructive. Say what is good about the patches, and how they could be improved, rather than just pointing out all the flaws. Of course, you should also be honest.
  • Be clear. Make it clear what action(s) are necessary to improve or correct the code. Clearly differentiate changes that must be made before you would consider merging the code from "nice to have" changes.
  • People are more important than code. Adjust the level and thoroughness of your comments to the committer. For long-time contributors, you can be more stringent. For new contributors, try hard to accept their patches as quickly as possible. Of course there always needs to be a basic level of correctness and style, but smaller issues can always be cleaned up later. The most important thing is for new contributors to have a positive experience, so that they want to continue contributing.
  • See it through. If you review a patch, try to stay with it through the process of revision and eventual merge. Don't demand some changes and then ignore subsequent updates. As a corollary, make sure your notification settings are such that you are aware when someone makes a new commit or comments on a pull request you have reviewed.
• Merge. When a pull request has been reviewed and the reviewers are happy with its current state (perhaps after one or more rounds of revision), merge it.

• Submit your own code for review. Having push access means you are trusted to make good commits, but you are still welcome (and encouraged, for larger commits) to submit pull requests so others can review your code. Whether to submit a pull request or just push in any given situation is left up to your judgment.

  Note that when submitting a pull request, you need not fork the repo; you are always welcome to create a new "topic branch" in the main repo and push to that branch. Then open a pull request from the branch to master (see [1]). If you prefer, you can also fork the repo and submit a pull request from your own feature branch.

• New features or breaking changes should be accompanied by documentation changes before being merged.
  • Any pull request that adds new functionality or makes breaking changes should not be merged until it is accompanied by a corresponding pull request to diagrams-doc.
  • The diagrams-doc pull request need not necessarily be complete, but should at least add placeholders in places where documentation should be added or changed. For example, for a new feature it would be enough to just add a section in the user manual with a todo container.
  • The accompanying -doc pull request does not have to be written by the same person as the feature pull request. If you implement a new feature and can interest someone else to write the documentation, great. If you are reviewing a pull request from a new contributor, consider writing the documentation (or at least inserting a placeholder) yourself, rather than placing an additional burden upon the new contributor.