This document describes the workflow used by Node.js contributors to contribute to the project as efficiently as possible. It starts by describing the most common workflow for issues and pull-requests. Then, it describes the most common use cases for contributors: how to find the next issue to work on that will maximize their impact on the project. Finally, it describes in greater details the set of tools and concepts that were mentioned in the first two sections.
In this document, the term "contributor" represents anybody who spends some time to improve the Node.js project. It isn't limited to core team members. When a distinction needs to be made, more specific terms such as "core team member" or "collaborator" are used.
We understand that work within an open source project with a large number of contributors needs to remain flexible. For this reason, this document describes recommended guidelines that will help all contributors to work more efficiently, but will not prevent valuable contributions from being acknowledged and integrated if they do not follow some items of these guidelines.
There is however two hard rules: using milestones and testing changes.
Before merging changes in the repository, they need to be associated to a pull-request that has been accepted in a specific milestone. For more details about nominating and accepting issues/pull-requests within a milestone, see the corresponding section.
Changes also are merged from pull requests' branches into Node.js' code repository only by the node-accept-pull-request Jenkins job. This ensures that all tests pass for every pull requests before being merged. This makes identifiying the root cause of tests failures easier since, at any given time, all tests pass on all branches.
This section presents what we consider to be the ideal workflow for issues and pull-requests. These guidelines are not hard, strict rules that need to be followed exactly as they are outlined.
- A new issue is filed against the joyent/node issues tracker.
- This issue is picked up by a contributor. She assigns the issue to herself and sets the
S-investigatinglabel. - When a full assessment is done, the
S-investigatinglabel is removed, and theS-readylabel is added. TheS-confirmed-buglabel is added to the issue if it describes a bug. A value for the effort and priority labels needs to be set. The issue is nominated for a specific milestone. The contributor who did the assessment can unassign herself from the issue to let other contributors pick it up for the next steps, or can stay assigned to this issue if she wants to fix it. - The issue is either accepted in a milestone or closed.
- At some point in time, a contributor starts implementing the code that fixes the issue. Any contributor is strongly encouraged to pick issues that have the
M-acceptedlabel and that belong to a given milestone. See how to pick an issue to fix for more details on how to pick an issue that has the greater impact on the project now. She assigns the issue to herself and sets theS-fixinglabel and removes theS-readylabel. - When the pull-request that fixes the issue has landed in the repository, the issue is closed.
There are two types of pull-requests:
- Pull-requests that fix a specific issue, or a specific set of issues.
- Spontaneous pull-requests that are not linked to any issue.
The workflows for these two types of pull-requests are slightly different. Even if they have a lot of similarities, they are described separately for the sake of clarity.
- A contributor can pick any issue to work on based on these guidelines, or can create a new one that describes the problem that this pull request fixes, or the functionality that it implements.
- She removes the
S-readylabel from the corresponding issue(s) if present, and sets theS-fixinglabel when she starts working on fixing them. - When the fix for the corresponding issue(s) is ready, she submits a pull-request. The issue number must be mentioned in the commit message. The
S-waiting-for-reviewlabel is set. - A core team member performs a thorough code review. The
S-waiting-for-reviewlabel is cleared. - If the code review does not pass, the author needs to make changes according to the code review. When changes are made, the
S-waiting-for-reviewlabel is set and we're back to step 3. - If the code review passes, the pull-request is nominated for the same milestone as the issue it fixes.
- If the pull-request is accepted in a milestone, the changes can be merged into the repository by using the appropriate Jenkins job that runs all tests if all tests pass.
- If some tests fail, the
S-tests-failinglabel is set and we're back to step 3. - If the pull-request is not accepted in any milestone, it can be closed.
- A contributor decides to make a spontaneous code contribution that is not tied to any issue and submits a pull-request. The
S-waiting-for-reviewlabel is set. - She nominates the pull-request for a given milestone.
- A core team member performs a thorough code review. The
S-waiting-for-reviewlabel is cleared. - If the code review does not pass, the author needs to make changes according to the code review. When changes are made, the
S-waiting-for-reviewlabel is set and we're back to step 3. - If the code review passes and if the pull-request is accepted in a milestone, the changes can be merged into the repository by using the appropriate Jenkins job that runs all tests if all tests pass.
- If some tests fail, the
S-tests-failinglabel is set and when the code is fixed to pass the tests, we're back to step 3. - If the pull-request is not accepted in any milestone, it can be closed.
When looking for issues to triage, one should look first for unassigned issues with the S-ready and S-investigating label not set.
If an issue has the S-ready label not set but the S-investigating label set, make sure that someone is assigned to this issue. If someone is assigned to this issue but hasn't updated it in a while, please mention the assignee and @joyent/node-coreteam and ask an update on the investigation process.
When looking for issues to fix, one should look first for unassigned issues with the S-ready and M-accepted labels set and the S-fixing label not set.
If an issue has the S-ready, M-accepted and S-fixing labels set but is unassigned, please mention @joyent/node-coreteam in the issue's comments to figure out what the state of this issue is.
The same should be done if an issue has the S-ready and M-accepted labels set and the S-fixing label is not set, but the issue is assigned.
When looking for pull-requests to review, one should primarily look for pull-requests within the next milestones with the M-accepted and S-waiting-for-review labels set. Higher priority pull-requests should be reviewed first.
The goal of using milestones is to make sure that the team is focused on what matters for the project today. It is the cornerstone of this workflow, and the only tool that must absolutely be used by every contributor.
Combined with the priority label and the assignee property, anyone can determine the most important issue to work on today by showing issues that match the following criteria:
- They belong to the next milestone and are accepted for this milestone (the
M-acceptedlabel is set). - They have the the highest priority level within this milestone.
- They are not assigned.
Also, when choosing their next issue to fix, contributors should only pick issues that are accepted in a milestone. However, anyone is encouraged to assess, or triage (reproduce, evaluate priority and effort) any issue, not only those that are accepted in a milestone. See the guidelines on how to pick what to work on next for more information.
For instance, when the team is focused on fixing bugs that are found in the v0.12.0 release. These bug fixes will be shipped in the v0.12.1 release. For this reason, most contributors should spend most of their time going through issues that belong to the 0.12.1 milestone and that match the criterias mentioned above.
At any given point in time, there are at least three milestones available:
- The current stable version milestone.
- The current maintenance version milestone.
- The current development version milestone.
At the time of writing, these corresponds respectively to the milestones 0.12.1, 0.10.37 and 0.13.1.
No pull-request can be merged into the repository without being accepted in the corresponding milestone first
The stable and maintenance milestones have a fundamental difference with the development milestone. Issues and pull-requests in the stable and maintenance milestones can only be accepted during the weekly milestone review process. Those in the development milestone can be accepted at any time by any core team member.
Towards the end of life of a development branch (for instance, for the last two development releases before a new stable version is released), development milestones can adopt the same requirements as stable/maintenance milestones. This facilitates the convergence of the development branch to a point where it can be released as a stable version.
After an issue has been assessed properly, if it doesn't belong to a milestone but should, it is nominated for that milestone before any work is done to fix it.
For pull-requests, they need to be nominated for a specific milestone before they can be accepted in this milestone, and then eventually have their changes integrated in the repository.
For a non core team member to nominate an issue or a pull-request for a milestone, add a comment of the following form:
@joyent/node-coreteam nominate:milestone-name
where milestone-name is a name of a milestone that can be found in the list of milestones.
For instance, nominate:0.12.1 would nominate an issue or pull-request for the 0.12.1 milestone. The associated changes would eventually be available in node.js' v0.12.1 release if the nomination is accepted by the core team.
After the @joyent/node-coreteam nominate: line, please provide a clear and concise explanation for why this issue/pull-request should be accepted in that milestone. This will be used by the core team when deciding whether or not to accept this issue into the target milestone.
For a core team member to nominate an issue/pull-request for a milestone, add it to the desired milestone and set the "M-nominated" tag. Add a comment that provides a clear and concise explanation for why this issue/pull-request should be accepted in that milestone. This will be used by the core team when deciding whether or not to accept this issue into the target milestone.
Sometimes, some issues need to be fixed and shipped in a maintenance or stable release in a more timely manner. For instance, security issues can have a greater impact than others, and sometimes need to be fixed as fast as possible.
For these issues, the guidelines described in the security page of the Node.js website apply, and the review process that goes on during the weekly milestones review meeting is carried out by the security process.
After an issue or a pull-request has been nominated, it needs to be accepted within that milestone before any associated change can be merged into the repository. The process of accepting issues and pull-requests is different depending on which milestone the issue/PR belongs to:
- For those belonging to a stable or maintenance milestone, the process takes place during the weekly milestones review meeting.
- For those belonging to a development milestone, any core team member can accept the issue/pull-request at any time.
Every Wednesday, the core team goes through all nominated issues in the stable and maintenance milestone and evaluate if they should be accepted for this milestone or moved into another milestone. If an issue is accepted for this milestone, the "M-nominated" tag is removed and the "M-accepted" tag is added.
Issues that are not accepted in the same milestone as the one it was nominated for can be moved to subsequent milestones or closed.
Sometime, milestones will be closed with some less priority issues/pull requests still open and nominated or accepted into that milestone. These issues will keep their nomination/acceptance status, but will be moved to the next corresponding maintenance/stable/development milestone.
For instance, an issue nominated or accepted for the 0.12.1 milestone when this milestone is closed will be automatically nominated/accepted for the 0.12.2 milestone.
On GitHub, issues can only be assigned to collaborators. Only a minority of users and contributors have the collaborator status in the GitHub project. Currently, collaborators are the members of the core team.
Issues and pull requests should be assigned to collaborators in charge of moving them forward. If an issue or a pull request has been created by an external contributor, it should be assigned to a collaborator who will coordinate the work that needs to be done to eventually close it.
Issues should be assigned to collaborators as soon as they start working (or coordinating work) on it, and should be unassigned as soon as they stop working (or coordinating work) on it. The goal is to make it easier for core team members to pickup new issues to work on or coordinate.
One issue can be assigned to one or more collaborators throughout its lifetime.
One collaborator can be assigned more than one issue. Issues should be spread evenly accross collaborators as much as possible.
The priority label is used to make sure that the team and all contributors are focused on what is important for the project today. In addition to a more systematic usage of milestones, this should make it much easier for anyone to identify what is the most priority issue to work on next.
P-0, P-1, P-2, and P-3, all exclusives. For a given milestone, all P-0, P-1 and P-2 issues should be fixed before closing it and releasing a new version of Node.js.
P-0 is used to label an issue that breaks node on at least one supported platform for a majority of users. It needs to be taken care of as soon as possible. An example of such an issue if for instance nodejs/node-v0.x-archive#8897. Basically, core team members need to make sure that at least one person is focused on any P-0 issue at any given time.
P-1 is a an issue that represents a significant regression but that doesn't break a majority of users on any supported platform. Anything that could block other contributors should also be labeled P-1, as it needs to be done as soon as possible to allow them to resume their normal workflow. V8 or libuv upgrades are good examples of such issues.
P-2 is an issue that represents a minor regression or a significant regression that impacts only a small minority of users.
P-3 is a nice to have.
The effort label represents an estimation of the time it would take to fix a given issue. E-1 means one hour, E-10 means ten hours and E-50 means fifty hours. These time estimates are of course approximate. These three values correspond respectively to a trivial issue, a moderately complex issue, and a complex issue. If a problem would take more than 50 hours to solve, it should probably be split into smaller problems.
E-1, E-10, E-50, all exclusive.
The status label is used to identify in which step of the workflow an issue or pull-request is. Some status apply only to issues, some apply only to pull-requests and finally some apply to both.
S-investigating.S-ready.S-confirmed-bug.S-need-more-info.S-fixing.S-maybe-close.
Following is a list of source states with their corresponding potential destination states where _ represents the initial state:
- _ ->
S-investigating. S-investigating->S-ready,S-confirmed-bug,S-need-more-info,S-maybe-close.S-ready->S-fixing.S-fixing->S-waiting-for-review.S-waiting-for-review->S-tests-failing, closed.
S-waiting-for-review.S-tests-failing.S-maybe-close.
Here's a list of source states with their corresponding potential destination states where _ represents the initial state:
- _ ->
S-waiting-for-review. S-waiting-for-review->S-tests-failing,S-maybe-close, closed.S-maybe-close-> closed, _.
When looking at an issue or a pull-request, sometimes one may not be able to assign a proper value to every label needed for the issue to be properly assessed. In this case, the form "PREFIX-", such as "P-" for "priority needs to be determined", or "E-" for "effort needs to be determined" can be used.
Contributors can search for every issue labeled "P-" and/or "E-" to find which ones need some help in completing an assessment.
A significant amount of time is spent by core team members testing and troubleshooting tests failing on platforms that are less popular among contributors, such as Windows and SmartOS.
Thus, before being landed, changes need to be tested on all supported platforms by using the node-accept-pull-request Jenkins job.
Starting the node-accept-pull-request Jenkins job currently requires to have an account on the Node.js' Jenkins platform. This is currently restricted to Node.js' core team members. The process is also manual, but the team is working on automating as much of this process as possible.
The reponsibility of triggering the tests and potentially merging changes is on the core team member who determines that a pull request is ready to be merged. A pull request is ready to be merged when it has been accepted into the current maintenance/development/stable milestone and at least one core team member approved the changes during a code review.
To start testing any pull-request, simply follow the following step:
- Point your broser to the node-accept-pull-request Jenkins job.
- On the left hand side, you should see "Build with parameters". If not, it probably means that you're not logged in. You need to be logged in to start this job.
- Click on "Build with parameters" and enter the pull-request number in the
PR_IDtext field. In theTARGET_BRANCHtext field, enter the name of the branch into which the changes in the pull-request would eventually be merged. For instance, enterv0.12for a PR that targets the v0.12 branch. In theREVIEWED_BYfield, enter the name and email address of the reviewer like following:Full Name <email address>. - Click on the "Build" button.
A new job number with a progress bar running should appear in the bottom left of the page, in the "build history" section. A colored ball appears on the left of the new job with the following possible colors:
- Blue for all tests passing.
- Yellow for all tests passing but flaky tests failing.
- Red for at least one non-flaky test failing.
If the color is blue or yellow, the changes will be merged in the branch TARGET_BRANCH.
If a pull-request fails to pass all tests on all platforms, the label S-tests-failing should be added to it and its associated changes cannot be merged into the repository. Always make sure to notify the author of the pull-request of tests results, and if they failed, always provide links to the results of the Jenkins job that illustrate the failures.