|
| 1 | +## Maintainers Guide |
| 2 | + |
| 3 | +This guide is intended for maintainers — anybody with commit access to one or |
| 4 | +more Developer Journey repositories. |
| 5 | + |
| 6 | +## Methodology: |
| 7 | + |
| 8 | +A master branch. This branch MUST be releasable at all times. Commits and |
| 9 | +merges against this branch MUST contain only bugfixes and/or security fixes. |
| 10 | +Maintenance releases are tagged against master. |
| 11 | + |
| 12 | +A develop branch. This branch contains your proposed changes. |
| 13 | + |
| 14 | +The remainder of this document details how to merge pull requests to the |
| 15 | +repositories. |
| 16 | + |
| 17 | +## Merge approval |
| 18 | + |
| 19 | +The project maintainers use LGTM (Looks Good To Me) in comments on the code |
| 20 | +review to indicate acceptance. A change requires LGTMs from two of the members |
| 21 | +of the [watson-journey-dev-admins](https://github.com/orgs/IBM/teams/watson-journey-dev-admins) |
| 22 | +team. If the code is written by a member, the change only requires one more |
| 23 | +LGTM. |
| 24 | + |
| 25 | +## Reviewing Pull Requests |
| 26 | + |
| 27 | +We recommend reviewing pull requests directly within GitHub. This allows a |
| 28 | +public commentary on changes, providing transparency for all users. When |
| 29 | +providing feedback be civil, courteous, and kind. Disagreement is fine, so |
| 30 | +long as the discourse is carried out politely. If we see a record of uncivil |
| 31 | +or abusive comments, we will revoke your commit privileges and invite you to |
| 32 | +leave the project. |
| 33 | + |
| 34 | +During your review, consider the following points: |
| 35 | + |
| 36 | +### Does the change have impact? |
| 37 | + |
| 38 | +While fixing typos is nice as it adds to the overall quality of the project, |
| 39 | +merging a typo fix at a time can be a waste of effort. |
| 40 | +(Merging many typo fixes because somebody reviewed the entire component, |
| 41 | +however, is useful!) Other examples to be wary of: |
| 42 | + |
| 43 | +Changes in variable names. Ask whether or not the change will make |
| 44 | +understanding the code easier, or if it could simply a personal preference |
| 45 | +on the part of the author. |
| 46 | + |
| 47 | +Essentially: feel free to close issues that do not have impact. |
| 48 | + |
| 49 | +### Do the changes make sense? |
| 50 | + |
| 51 | +If you do not understand what the changes are or what they accomplish, |
| 52 | +ask the author for clarification. Ask the author to add comments and/or |
| 53 | +clarify test case names to make the intentions clear. |
| 54 | + |
| 55 | +At times, such clarification will reveal that the author may not be using |
| 56 | +the code correctly, or is unaware of features that accommodate their needs. |
| 57 | +If you feel this is the case, work up a code sample that would address the |
| 58 | +issue for them, and feel free to close the issue once they confirm. |
| 59 | + |
| 60 | +### Is this a new feature? If so: |
| 61 | + |
| 62 | +Does the issue contain narrative indicating the need for the feature? If not, |
| 63 | +ask them to provide that information. Since the issue will be linked in the |
| 64 | +changelog, this will often be a user's first introduction to it. |
| 65 | + |
| 66 | +Are new unit tests in place that test all new behaviors introduced? If not, do |
| 67 | +not merge the feature until they are! |
| 68 | +Is documentation in place for the new feature? (See the documentation |
| 69 | +guidelines). If not do not merge the feature until it is! |
| 70 | +Is the feature necessary for general use cases? Try and keep the scope of any |
| 71 | +given component narrow. If a proposed feature does not fit that scope, |
| 72 | +recommend to the user that they maintain the feature on their own, and close |
| 73 | +the request. You may also recommend that they see if the feature gains traction |
| 74 | +amongst other users, and suggest they re-submit when they can show such support. |
0 commit comments