|
| 1 | +# Maintainers Guide |
| 2 | + |
| 3 | +This guide is intended for maintainers - anybody with commit access to one or |
| 4 | +more Code Pattern repositories. |
| 5 | + |
| 6 | +## Methodology |
| 7 | + |
| 8 | +This repository does not have a traditional release management cycle, but |
| 9 | +should instead be maintained as a useful, working, and polished reference at |
| 10 | +all times. While all work can therefore be focused on the master branch, the |
| 11 | +quality of this branch should never be compromised. |
| 12 | + |
| 13 | +The remainder of this document details how to merge pull requests to the |
| 14 | +repositories. |
| 15 | + |
| 16 | +## Merge approval |
| 17 | + |
| 18 | +The project maintainers use LGTM (Looks Good To Me) in comments on the pull |
| 19 | +request to indicate acceptance prior to merging. A change requires LGTMs from |
| 20 | +two project maintainers. If the code is written by a maintainer, the change |
| 21 | +only requires one additional LGTM. |
| 22 | + |
| 23 | +## Reviewing Pull Requests |
| 24 | + |
| 25 | +We recommend reviewing pull requests directly within GitHub. This allows a |
| 26 | +public commentary on changes, providing transparency for all users. When |
| 27 | +providing feedback be civil, courteous, and kind. Disagreement is fine, so long |
| 28 | +as the discourse is carried out politely. If we see a record of uncivil or |
| 29 | +abusive comments, we will revoke your commit privileges and invite you to leave |
| 30 | +the project. |
| 31 | + |
| 32 | +During your review, consider the following points: |
| 33 | + |
| 34 | +### Does the change have positive impact? |
| 35 | + |
| 36 | +Some proposed changes may not represent a positive impact to the project. Ask |
| 37 | +whether or not the change will make understanding the code easier, or if it |
| 38 | +could simply be a personal preference on the part of the author (see |
| 39 | +[bikeshedding](https://en.wiktionary.org/wiki/bikeshedding)). |
| 40 | + |
| 41 | +Pull requests that do not have a clear positive impact should be closed without |
| 42 | +merging. |
| 43 | + |
| 44 | +### Do the changes make sense? |
| 45 | + |
| 46 | +If you do not understand what the changes are or what they accomplish, ask the |
| 47 | +author for clarification. Ask the author to add comments and/or clarify test |
| 48 | +case names to make the intentions clear. |
| 49 | + |
| 50 | +At times, such clarification will reveal that the author may not be using the |
| 51 | +code correctly, or is unaware of features that accommodate their needs. If you |
| 52 | +feel this is the case, work up a code sample that would address the pull |
| 53 | +request for them, and feel free to close the pull request once they confirm. |
| 54 | + |
| 55 | +### Does the change introduce a new feature? |
| 56 | + |
| 57 | +For any given pull request, ask yourself "is this a new feature?" If so, does |
| 58 | +the pull request (or associated issue) contain narrative indicating the need |
| 59 | +for the feature? If not, ask them to provide that information. |
| 60 | + |
| 61 | +Are new unit tests in place that test all new behaviors introduced? If not, do |
| 62 | +not merge the feature until they are! Is documentation in place for the new |
| 63 | +feature? (See the documentation guidelines). If not do not merge the feature |
| 64 | +until it is! Is the feature necessary for general use cases? Try and keep the |
| 65 | +scope of any given component narrow. If a proposed feature does not fit that |
| 66 | +scope, recommend to the user that they maintain the feature on their own, and |
| 67 | +close the request. You may also recommend that they see if the feature gains |
| 68 | +traction among other users, and suggest they re-submit when they can show such |
| 69 | +support. |
0 commit comments