|
1 |
| -## Maintainers Guide |
| 1 | +# Maintainers Guide |
2 | 2 |
|
3 |
| -This guide is intended for maintainers — anybody with commit access to one or |
4 |
| -more Developer Journey repositories. |
| 3 | +This guide is intended for maintainers - anybody with commit access to one or |
| 4 | +more Code Pattern repositories. |
5 | 5 |
|
6 |
| -## Methodology: |
| 6 | +## Methodology |
7 | 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. |
| 8 | +This repository does not have a traditional release management cycle, but |
| 9 | +should instead be maintained as 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. |
13 | 12 |
|
14 | 13 | The remainder of this document details how to merge pull requests to the
|
15 | 14 | repositories.
|
16 | 15 |
|
17 | 16 | ## Merge approval
|
18 | 17 |
|
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. |
| 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. |
24 | 22 |
|
25 | 23 | ## Reviewing Pull Requests
|
26 | 24 |
|
27 | 25 | We recommend reviewing pull requests directly within GitHub. This allows a
|
28 | 26 | 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. |
| 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. |
33 | 31 |
|
34 | 32 | During your review, consider the following points:
|
35 | 33 |
|
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: |
| 34 | +### Does the change have positive impact? |
42 | 35 |
|
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. |
| 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)). |
46 | 40 |
|
47 |
| -Essentially: feel free to close issues that do not have impact. |
| 41 | +Pull requests that do not have a clear positive impact should be closed without |
| 42 | +merging. |
48 | 43 |
|
49 | 44 | ### Do the changes make sense?
|
50 | 45 |
|
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. |
| 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. |
54 | 49 |
|
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. |
| 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. |
59 | 54 |
|
60 |
| -### Is this a new feature? If so: |
| 55 | +### Does the change introduce a new feature? |
61 | 56 |
|
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. |
| 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. |
65 | 60 |
|
66 | 61 | 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. |
| 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