-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Merging PRs
Luis Lavena edited this page Jul 11, 2017
·
5 revisions
- Do not directly commit to
master, always create a branch. - If a crystal-lang org member creates a PR, at least one other member has to approve it before it is merged. In most of these cases the approver will just merge it. In some cases they may not feel sure to merge even when it looks good to them: in those cases it is totally ok to leave the PR approved and ask for another member to look at it for further assurance.
- If a contributor who's not a member of the crystal-lang org creates a PR, at least two members of the crystal-lang org have to approve it before it is merged. This is not because of over jealousy: it is to guarantee that the knowledge about the change doesn't lie in just one person.
- It must have passed CI.
- Every new public API has to include some minimum documentation: params (and particularly interesting combinations of them if the method is complex), results, interesting, self-contained examples. By public API here we mean any method or member that can be accessed from outside the module/class/struct, etc.
- Unless the PR is just a refactor or a documentation change, it must include changes to the specs that reflect and check the new behaviors.
- If the change affects the compiler, run a couple of benchmarks to check how it affects compiler performance. If it affects the compiler negatively, involve other members in the discussion to ensure that the change is worth the performance penalty.