|
| 1 | +# Workflow guidelines |
| 2 | + |
| 3 | +## Introduction |
| 4 | + |
| 5 | +This document describes the workflow used by Node.js contributors to |
| 6 | +contribute to the project as efficiently as possible. It starts by describing |
| 7 | +the most common workflow for issues and pull-requests. Then, it describes the |
| 8 | +most common use cases for contributors: how to find the next issue to work on |
| 9 | +that will maximize their impact on the project. Finally, it describes in |
| 10 | +greater details the set of tools and concepts that were mentioned in the first |
| 11 | +two sections. |
| 12 | + |
| 13 | +In this document, the term "contributor" represents anybody who spends some |
| 14 | +time to improve the Node.js project. It isn't limited to core team members. |
| 15 | +When a distinction needs to be made, more specific terms such as "core team |
| 16 | +member" or "collaborator" are used. |
| 17 | + |
| 18 | +## Flexible workflow, only two strict rules |
| 19 | + |
| 20 | +We understand that work within an open source project with a large number of |
| 21 | +contributors needs to remain flexible. For this reason, most of the items |
| 22 | +described in this document are optional. There are only two hard, mandatory |
| 23 | +rules: |
| 24 | + |
| 25 | +1. __Using milestones__. Before merging changes in the repository, they _need_ to |
| 26 | +be associated to a pull-request that has been _added_ to a specific milestone. |
| 27 | +For more details about adding issues/pull requests to a milestone, see [the |
| 28 | +corresponding |
| 29 | +section](#index_md_adding_issues_and_pull_requests_to_a_milestone). |
| 30 | + |
| 31 | +2. __Testing changes__. Changes are merged from pull requests' branches into |
| 32 | +Node.js' code repository _only_ by [the node-accept-pull-request Jenkins |
| 33 | +job](#index_md_using_jenkins_to_build_test_and_merge_every_pr). This ensures |
| 34 | +that __all tests pass for every pull requests before being merged__. This |
| 35 | +makes identifiying the root cause of tests failures easier since, at any given |
| 36 | +time, all tests pass on all branches. |
| 37 | + |
| 38 | +The rest of the guidelines are optional. Following these optional recommended |
| 39 | +guidelines will help all contributors to work more efficiently. However, not |
| 40 | +following them will not prevent valuable contributions from being acknowledged |
| 41 | +and integrated. |
| 42 | + |
| 43 | +## Ideal life cycle of issues and pull-requests |
| 44 | + |
| 45 | +This section presents what we consider to be the ideal workflow for issues and |
| 46 | +pull-requests. These guidelines are not hard, strict rules that need to be |
| 47 | +followed exactly as they are outlined. |
| 48 | + |
| 49 | +### Ideal life cycle of a GitHub issue |
| 50 | + |
| 51 | +1. A new issue is filed against the joyent/node issues tracker. |
| 52 | + |
| 53 | +2. This issue is picked up by a contributor. She assigns the issue to herself if |
| 54 | +she's a collaborator, or leaves it unassigned. |
| 55 | + |
| 56 | +3. When a full assessment is done, a comment is added to the issue that describes |
| 57 | +its impact on the project. The `S-confirmed-bug` label is added to the issue |
| 58 | +if it describes an actual bug. |
| 59 | + |
| 60 | +4. The issue can be added [in a milestone](#index_md_adding_issues_and_pull_requests_to_a_milestone), |
| 61 | +left without a milestone or closed. |
| 62 | + |
| 63 | +5. If the issue was added to a milestone, at some point in time, a contributor |
| 64 | +starts implementing the code that fixes the issue. Any contributor is strongly |
| 65 | +encouraged to pick issues that belong to a given milestone. See [how to pick |
| 66 | +an issue to fix](#index_md_picking_issues_to_fix) for more details on how to pick |
| 67 | +an issue that has the greater impact on the project now. A collaborator |
| 68 | +assigns the issue to herself if she's fixing the issue or to coordinate the |
| 69 | +work done by an external contributor to fix it. |
| 70 | + |
| 71 | +6. When the pull-request that fixes the issue has landed in the repository, the |
| 72 | +issue is closed. |
| 73 | + |
| 74 | +### Ideal life cycle of a GitHub pull-request |
| 75 | + |
| 76 | +There are two types of pull-requests: |
| 77 | +* Pull-requests that fix a specific issue, or a specific set of issues. |
| 78 | +* Spontaneous pull-requests that are not linked to any issue. |
| 79 | + |
| 80 | +The workflows for these two types of pull-requests are slightly different. |
| 81 | +Even if they have a lot of similarities, they are described separately for the |
| 82 | +sake of clarity. |
| 83 | + |
| 84 | +#### Pull-requests that fix (an) exiting issue(s) |
| 85 | + |
| 86 | +1. A contributor can pick any issue to work on based on these [guidelines](#index_md_how_ |
| 87 | +to_pick_an_issue_to_fix), or can create a new one that describes the problem |
| 88 | +that this pull request fixes, or the functionality that it implements. |
| 89 | + |
| 90 | +2. When the fix for the corresponding issue(s) is ready, she submits a pull- |
| 91 | +request. The issue number must be mentioned in the commit message. |
| 92 | + |
| 93 | +3. A core team member performs a thorough code review. |
| 94 | + |
| 95 | +4. If the code review does not pass, the author needs to make changes according |
| 96 | +to the code review. When changes are made, we're back to step 3. |
| 97 | + |
| 98 | +6. If the code review passes, the pull-request is added to the same milestone as |
| 99 | +the issue it fixes. |
| 100 | + |
| 101 | +7. The changes are merged into the repository by [using the appropriate Jenkins |
| 102 | +job that runs all tests](#index_md_using_jenkins_to_build_test_and_merge_every_pr). |
| 103 | + |
| 104 | +8. If some tests fail, we're back to step 3. |
| 105 | + |
| 106 | +#### Spontaneous pull-requests not tied to any issue |
| 107 | + |
| 108 | +1. A contributor decides to make a spontaneous code contribution that is not tied |
| 109 | +to any issue and submits a pull-request. |
| 110 | + |
| 111 | +2. At some point, a collaborator [adds the pull-request to a given milestone |
| 112 | +](#index_md_adding_issues_and_pull_requests_to_a_milestone). |
| 113 | + |
| 114 | +3. A core team member performs a thorough code review. |
| 115 | + |
| 116 | +4. If the code review does not pass, the author needs to make changes according |
| 117 | +to the code review. When changes are made, we're back to step 3. |
| 118 | + |
| 119 | +5. If the code review passes and if the pull-request is [added in a milestone |
| 120 | +](#index_md_adding_issues_and_pull_requests_to_a_milestone), the changes can be merged |
| 121 | +into the repository by [using the appropriate Jenkins job that runs all tests |
| 122 | +](#index_md_using_jenkins_to_build_test_and_merge_every_pr). |
| 123 | + |
| 124 | +6. If some tests fail, someone needs to fix them and, when it's done, we're back |
| 125 | +to step 3. |
| 126 | + |
| 127 | +## How to pick what to work on next? |
| 128 | + |
| 129 | +### Picking issues to triage |
| 130 | + |
| 131 | +When looking for issues to triage, one should look first for _unassigned_ |
| 132 | +issues that do not belong to a milestone. |
| 133 | + |
| 134 | +If someone is assigned to this issue but hasn't updated it in a while, please |
| 135 | +mention the assignee and @joyent/node-coreteam and ask an update on the |
| 136 | +investigation process. |
| 137 | + |
| 138 | +### Picking issues to fix |
| 139 | + |
| 140 | +When looking for issues to fix, one should look first for _unassigned_ issues |
| 141 | +that belong to the [next stable or development milestone](#index_md_systematic_usage_ |
| 142 | +of_milestones) and that have the `S-confirmed-bug` label set. [Higher |
| 143 | +priority](#index_md_priority) issues should be fixed first. |
| 144 | + |
| 145 | +If someone is assigned to this issue but hasn't updated it in a while, please |
| 146 | +mention the assignee and @joyent/node-coreteam and ask an update on the fixing |
| 147 | +process. |
| 148 | + |
| 149 | +### Picking pull-requests to review |
| 150 | + |
| 151 | +When looking for pull-requests to review, one should primarily look for pull- |
| 152 | +requests within [the next milestones](#index_md_systematic_usage_of_milestones). |
| 153 | +[Higher priority](#index_md_priority) pull-requests should be reviewed first. |
| 154 | + |
| 155 | +## Workflow tools |
| 156 | + |
| 157 | +### Systematic usage of milestones |
| 158 | + |
| 159 | +The goal of using milestones is to make sure that the team is focused on what |
| 160 | +matters for the project today. It is the cornerstone of this workflow, and the |
| 161 | +only tool that must absolutely be used by every contributor. |
| 162 | + |
| 163 | +Combined with the [priority label](#index_md_priority) and the [assignee property |
| 164 | +](#index_md_assigning_issues), anyone can determine the most important issue to work on |
| 165 | +today by showing issues that match the following criteria: |
| 166 | + |
| 167 | +* They belong to the next stable or development milestone. |
| 168 | +* They have the the highest priority level within this milestone. |
| 169 | +* They are not assigned. |
| 170 | + |
| 171 | +Also, when choosing their next issue to fix, contributors should only pick |
| 172 | +issues that belong to a milestone. However, anyone is encouraged to _assess_, |
| 173 | +or triage (reproduce, evaluate priority) any issue, not only those that belong |
| 174 | +to a milestone. See the guidelines on [how to pick what to work on next](#index_md_how_ |
| 175 | +to_pick_what_to_work_on_next) for more information. |
| 176 | + |
| 177 | +For instance, when the team is focused on fixing bugs that are found in the |
| 178 | +v0.12.0 release. These bug fixes will be shipped in the v0.12.1 release. For |
| 179 | +this reason, most contributors should spend most of their time going through |
| 180 | +issues that belong to [the 0.12.1 |
| 181 | +milestone](https://github.com/joyent/node/milestones/0.12.1) and that match |
| 182 | +the criterias mentioned above. |
| 183 | + |
| 184 | +### Available milestones |
| 185 | + |
| 186 | +At any given point in time, there are at least three milestones available: |
| 187 | +* The current stable version milestone. |
| 188 | +* The current maintenance version milestone. |
| 189 | +* The current development version milestone. |
| 190 | + |
| 191 | +At the time of writing, these corresponds respectively to the milestones |
| 192 | +0.12.1, 0.10.37 and 0.13.1. |
| 193 | + |
| 194 | +__No pull-request can be merged into the repository without being added in the |
| 195 | +corresponding milestone first__ |
| 196 | + |
| 197 | +#### Adding issues and pull-requests to a milestone |
| 198 | + |
| 199 | +After an issue has been assessed properly, if it doesn't belong to a milestone |
| 200 | +but should, it is added to that milestone by a collaborator before any work is |
| 201 | +done to fix it. |
| 202 | + |
| 203 | +##### Non core team members |
| 204 | + |
| 205 | +Non core members can nominate an issue or a pull request for a milestone if |
| 206 | +they want. To nominate an issue, add a comment of the following form: |
| 207 | +``` |
| 208 | +@joyent/node-coreteam nominate:milestone-name |
| 209 | +``` |
| 210 | +where `milestone-name` is a name of a milestone that can be found in [the list |
| 211 | +of milestones](https://github.com/joyent/node/milestones). |
| 212 | + |
| 213 | +For instance, `nominate:0.12.1` would nominate an issue or pull-request for |
| 214 | +the 0.12.1 milestone. The associated changes would eventually be available in |
| 215 | +node.js' v0.12.1 release if the nomination is accepted by the core team. |
| 216 | + |
| 217 | +After the `@joyent/node-coreteam nominate:` line, please provide a clear and |
| 218 | +concise explanation for why this issue/pull-request should be accepted in that |
| 219 | +milestone. This will be used by the core team when deciding whether or not to |
| 220 | +add this issue into the target milestone. |
| 221 | + |
| 222 | +##### Core team members |
| 223 | + |
| 224 | +For a core team member to add an issue/pull-request to a milestone, simply add |
| 225 | +it to the desired milestone. Add a comment that provides a clear and concise |
| 226 | +explanation for why this issue/pull request should be accepted in that |
| 227 | +milestone and __mention @joyent/node-coreteam__. That allows anyone from the |
| 228 | +core team who disagrees with adding this issue/pull request into that |
| 229 | +milestone to express their opinion and suggest another option. |
| 230 | + |
| 231 | +#### Moving issues/pull requests when a milestone is closed |
| 232 | + |
| 233 | +Sometime, milestones will be closed with some less priority issues/pull |
| 234 | +requests still open. These issues/pull requests will be moved to the next |
| 235 | +corresponding maintenance/stable/development milestone. |
| 236 | + |
| 237 | +For instance, an open issue that belongs to the 0.12.1 milestone when this |
| 238 | +milestone is closed will be automatically added to the 0.12.2 milestone. |
| 239 | + |
| 240 | +## Assigning issues |
| 241 | + |
| 242 | +On GitHub, issues can only be assigned to collaborators. Only a minority of |
| 243 | +users and contributors have the collaborator status in the GitHub project. |
| 244 | +Currently, collaborators are [the members of the core |
| 245 | +team](https://nodejs.org/about/core-team/). |
| 246 | + |
| 247 | +Issues and pull requests should be assigned to collaborators in charge of |
| 248 | +moving them forward. If an issue or a pull request has been created by an |
| 249 | +external contributor, it should be assigned to a collaborator who will |
| 250 | +coordinate the work that needs to be done to eventually close it. |
| 251 | + |
| 252 | +Issues should be assigned to collaborators as soon as they start working (or |
| 253 | +coordinating work) on it, and should be unassigned as soon as they stop |
| 254 | +working (or coordinating work) on it. The goal is to make it easier for core |
| 255 | +team members to pickup new issues to work on or coordinate. |
| 256 | + |
| 257 | +One issue can be assigned to one or more collaborators throughout its |
| 258 | +lifetime. |
| 259 | + |
| 260 | +One collaborator can be assigned more than one issue. Issues should be spread |
| 261 | +evenly accross collaborators as much as possible. |
| 262 | + |
| 263 | +## Usage of labels |
| 264 | + |
| 265 | +### Priority |
| 266 | + |
| 267 | +The priority label is used to make sure that the team and all contributors are |
| 268 | +focused on what is important for the project today. In addition to a more |
| 269 | +systematic usage of milestones, this should make it much easier for anyone to |
| 270 | +identify what is the most priority issue to work on next. |
| 271 | + |
| 272 | +#### Values |
| 273 | + |
| 274 | +`P-0`, `P-1`, `P-2`, and `P-3`, all exclusives. For a given |
| 275 | +[milestone](#index_md_systematic_usage_of_milestones), all P-0, P-1 and P-2 issues should |
| 276 | +be fixed before closing it and releasing a new version of Node.js. |
| 277 | + |
| 278 | +`P-0` is used to label an issue that breaks node on at least one supported |
| 279 | +platform for a majority of users. It needs to be taken care of as soon as |
| 280 | +possible. An example of such an issue if for instance |
| 281 | +https://github.com/joyent/node/issues/8897. Basically, core team members need |
| 282 | +to make sure that at least one person is focused on any `P-0` issue at any |
| 283 | +given time. |
| 284 | + |
| 285 | +`P-1` is a an issue that represents a significant regression but that doesn't |
| 286 | +break a majority of users on any supported platform. Anything that could block |
| 287 | +other contributors should also be labeled `P-1`, as it needs to be done as |
| 288 | +soon as possible to allow them to resume their normal workflow. V8 or libuv |
| 289 | +upgrades are good examples of such issues. |
| 290 | + |
| 291 | +`P-2` is an issue that represents a minor regression or a significant regression |
| 292 | +that impacts only a small minority of users. |
| 293 | + |
| 294 | +`P-3` is a nice to have. |
| 295 | + |
| 296 | +### Status |
| 297 | + |
| 298 | +The status label is used to identify in which step of the workflow an issue or |
| 299 | +pull-request is. Some status apply only to issues, some apply only to pull- |
| 300 | +requests and finally some apply to both. |
| 301 | + |
| 302 | +#### Issues status |
| 303 | + |
| 304 | +* `S-confirmed-bug`. |
| 305 | +* `S-need-more-info`. |
| 306 | +* `S-maybe-close`. |
| 307 | + |
| 308 | +#### Pull-requests status |
| 309 | + |
| 310 | +* `S-maybe-close`. |
| 311 | + |
| 312 | +### Using Jenkins to build, test and merge every PR |
| 313 | + |
| 314 | +A significant amount of time is spent by core team members testing and |
| 315 | +troubleshooting tests failing on platforms that are less popular among |
| 316 | +contributors, such as Windows and SmartOS. |
| 317 | + |
| 318 | +Thus, before being landed, changes need to be tested on all supported |
| 319 | +platforms by using [the node-accept-pull-request Jenkins |
| 320 | +job](http://jenkins.nodejs.org/job/node-accept-pull-request/). |
| 321 | + |
| 322 | +Starting [the node-accept-pull-request Jenkins |
| 323 | +job](http://jenkins.nodejs.org/job/node-accept-pull-request/) currently |
| 324 | +requires to have an account on [the Node.js' Jenkins |
| 325 | +platform](http://jenkins.nodejs.org). This is currently restricted to Node.js' |
| 326 | +core team members. The process is also manual, but the team is working on |
| 327 | +automating as much of this process as possible. |
| 328 | + |
| 329 | +The reponsibility of triggering the tests and potentially merging changes is |
| 330 | +on the core team member who determines that a pull request is ready to be |
| 331 | +merged. A pull request is ready to be merged when [it has been added into the |
| 332 | +current maintenance/development/stable milestone](#index_md_systematic_usage_of_ |
| 333 | +milestones) and at least one core team member approved the changes during a |
| 334 | +code review. |
| 335 | + |
| 336 | +To start testing any pull-request, simply follow the following step: |
| 337 | + |
| 338 | +1. Point your browser to [the node-accept-pull-request Jenkins |
| 339 | +job](http://jenkins.nodejs.org/job/node-accept-pull-request/). |
| 340 | + |
| 341 | +2. On the left hand side, you should see "Build with parameters". If not, it |
| 342 | +probably means that you're not logged in. You need to be logged in to start |
| 343 | +this job. |
| 344 | + |
| 345 | +3. Click on "Build with parameters" and enter the pull-request number in the |
| 346 | +`PR_ID` text field. In the `TARGET_BRANCH` text field, enter the name of the |
| 347 | +branch into which the changes in the pull-request would eventually be merged. |
| 348 | +For instance, enter `v0.12` for a PR that targets the v0.12 branch. In the |
| 349 | +`REVIEWED_BY` field, enter the name and email address of the reviewer like |
| 350 | +following: `Full Name <email address>`. |
| 351 | + |
| 352 | +4. Click on the "Build" button. |
| 353 | + |
| 354 | +A new job number with a progress bar running should appear in the bottom left |
| 355 | +of the page, in the "build history" section. A colored ball appears on the |
| 356 | +left of the new job with the following possible colors: |
| 357 | + |
| 358 | +* Blue for all tests passing. |
| 359 | +* Yellow for all tests passing but flaky tests failing. |
| 360 | +* Red for at least one non-flaky test failing. |
| 361 | + |
| 362 | +If the color is blue or yellow, the changes will be merged in the branch |
| 363 | +`TARGET_BRANCH`. |
| 364 | + |
| 365 | +If a pull-request fails to pass all tests on all platforms, its associated |
| 366 | +changes cannot be merged into the repository. Always make sure to notify the |
| 367 | +author of the pull-request of tests results, and if they failed, always |
| 368 | +provide links to the results of the Jenkins job that illustrate the failures. |
0 commit comments