• Home
  • Line#
  • Scopes#
  • Navigate#
  • Raw
  • Download
1# Onboarding
2
3This document is an outline of the things we tell new collaborators at their
4onboarding session.
5
6## One week before the onboarding session
7
8* If the new Collaborator is not yet a member of the nodejs GitHub organization,
9  confirm that they are using [two-factor authentication][]. It will not be
10  possible to add them to the organization if they are not using two-factor
11  authentication. If they cannot receive SMS messages from GitHub, try
12  [using a TOTP mobile app][].
13* Suggest the new Collaborator install [`node-core-utils`][] and
14  [set up the credentials][] for it.
15
16## Fifteen minutes before the onboarding session
17
18* Prior to the onboarding session, add the new Collaborator to
19  [the collaborators team](https://github.com/orgs/nodejs/teams/collaborators).
20* Ask them if they want to join any [subsystem teams](https://github.com/orgs/nodejs/teams/core/teams)
21  and add them accordingly. See [Who to CC in the issue tracker][who-to-cc].
22
23## Onboarding session
24
25* This session will cover:
26  * [local setup](#local-setup)
27  * [project goals and values](#project-goals-and-values)
28  * [managing the issue tracker](#managing-the-issue-tracker)
29  * [reviewing pull requests](#reviewing-pull-requests)
30  * [landing pull requests](#landing-pull-requests)
31
32## Local setup
33
34* git:
35  * Make sure you have whitespace=fix: `git config --global --add
36    apply.whitespace fix`
37  * Always create a branch in your own GitHub fork for pull requests
38    * Branches in the `nodejs/node` repository are only for release lines
39  * Add the canonical nodejs repository as `upstream` remote:
40    * `git remote add upstream git@github.com:nodejs/node.git`
41  * To update from `upstream`:
42    * `git checkout main`
43    * `git fetch upstream HEAD`
44    * `git reset --hard FETCH_HEAD`
45  * Make a new branch for each pull request you submit.
46  * Membership: Consider making your membership in the Node.js GitHub
47    organization public. This makes it easier to identify collaborators.
48    Instructions on how to do that are available at
49    [Publicizing or hiding organization membership][].
50
51* Notifications:
52  * Use <https://github.com/notifications> or
53    set up email
54  * Watching the main repository will flood your inbox (several hundred
55    notifications on typical weekdays), so be prepared
56
57The project has a venue for real-time discussion:
58
59* [`#nodejs-core`](https://openjs-foundation.slack.com/archives/C019Y2T6STH) on
60  the [OpenJS Foundation Slack](https://slack-invite.openjsf.org/)
61
62## Project goals and values
63
64* Collaborators are the collective owners of the project
65  * The project has the goals of its contributors
66
67* There are some higher-level goals and values
68  * Empathy towards users matters (this is in part why we onboard people)
69  * Generally: try to be nice to people!
70  * The best outcome is for people who come to our issue tracker to feel like
71    they can come back again.
72
73* You are expected to follow _and_ hold others accountable to the
74  [Code of Conduct][].
75
76## Managing the issue tracker
77
78* You have (mostly) free rein; don't hesitate to close an issue if you are
79  confident that it should be closed.
80  * Be nice about closing issues! Let people know why, and that issues and pull
81    requests can be reopened if necessary.
82
83* See [Labels][].
84  * There is [a bot](https://github.com/nodejs-github-bot/github-bot) that
85    applies subsystem labels (for example, `doc`, `test`, `assert`, or `buffer`)
86    so that we know what parts of the code base the pull request modifies. It is
87    not perfect, of course. Feel free to apply relevant labels and remove
88    irrelevant labels from pull requests and issues.
89  * `semver-{minor,major}`:
90    * If a change has the remote _chance_ of breaking something, use the
91      `semver-major` label
92    * When adding a `semver-*` label, add a comment explaining why you're adding
93      it. Do it right away so you don't forget!
94  * Please add the [`author-ready`][] label for pull requests, if applicable.
95
96* See [Who to CC in the issue tracker][who-to-cc].
97  * This will come more naturally over time
98  * For many of the teams listed there, you can ask to be added if you are
99    interested
100    * Some are WGs with some process around adding people, others are only there
101      for notifications
102
103* When a discussion gets heated, you can request that other collaborators keep
104  an eye on it by opening an issue at the private
105  [nodejs/moderation](https://github.com/nodejs/moderation) repository.
106  * This is a repository to which all members of the `nodejs` GitHub
107    organization (not just collaborators on Node.js core) have access. Its
108    contents should not be shared externally.
109  * Node.js has a moderation team which you should contact when unsure
110    about taking action in the Node.js org.
111  * You can moderate non-collaborator posts yourself. Please
112    report the moderation action taken in accordance to the moderation
113    policy.
114  * You can always refer to the
115    [full moderation policy](https://github.com/nodejs/admin/blob/main/Moderation-Policy.md).
116  * You can contact someone in the
117    [full list of moderation team members](https://github.com/nodejs/moderation#moderation-team-members).
118
119## Reviewing pull requests
120
121* The primary goal is for the codebase to improve.
122
123* Secondary (but not far off) is for the person submitting code to succeed. A
124  pull request from a new contributor is an opportunity to grow the community.
125
126* Review a bit at a time. Do not overwhelm new contributors.
127  * It is tempting to micro-optimize. Don't succumb to that temptation. We
128    change V8 often. Techniques that provide improved performance today may be
129    unnecessary in the future.
130
131* Be aware: Your opinion carries a lot of weight!
132
133* Nits (requests for small changes that are not essential) are fine, but try to
134  avoid stalling the pull request.
135  * Identify them as nits when you comment: `Nit: change foo() to bar().`
136  * If they are stalling the pull request, fix them yourself on merge.
137
138* Insofar as possible, issues should be identified by tools rather than human
139  reviewers. If you are leaving comments about issues that could be identified
140  by tools but are not, consider implementing the necessary tooling.
141
142* Minimum wait for comments time
143  * There is a minimum waiting time which we try to respect for non-trivial
144    changes so that people who may have important input in such a distributed
145    project are able to respond.
146  * For non-trivial changes, leave the pull request open for at least 48 hours.
147  * If a pull request is abandoned, check if they'd mind if you took it over
148    (especially if it just has nits left).
149
150* Approving a change
151  * Collaborators indicate that they have reviewed and approve of the changes in
152    a pull request using GitHub's approval interface
153  * Some people like to comment `LGTM` (“Looks Good To Me”)
154  * You have the authority to approve any other collaborator's work.
155  * You cannot approve your own pull requests.
156  * When explicitly using `Changes requested`, show empathy – comments will
157    usually be addressed even if you don't use it.
158    * If you do, it is nice if you are available later to check whether your
159      comments have been addressed
160    * If you see that the requested changes have been made, you can clear
161      another collaborator's `Changes requested` review.
162    * Use `Changes requested` to indicate that you are considering some of your
163      comments to block the pull request from landing.
164
165* What belongs in Node.js:
166  * Opinions vary – it's good to have a broad collaborator base for that reason!
167  * If Node.js itself needs it (due to historical reasons), then it belongs in
168    Node.js.
169    * That is to say, `url` is there because of `http`, `freelist` is there
170      because of `http`, etc.
171  * Things that cannot be done outside of core, or only with significant pain
172    such as `async_hooks`.
173
174* Continuous Integration (CI) Testing:
175  * <https://ci.nodejs.org/>
176    * It is not automatically run. You need to start it manually.
177  * Log in on CI is integrated with GitHub. Try to log in now!
178  * You will be using `node-test-pull-request` most of the time. Go there now!
179    * Consider bookmarking it: <https://ci.nodejs.org/job/node-test-pull-request/>
180  * To get to the form to start a job, click on `Build with Parameters`. (If you
181    don't see it, that probably means you are not logged in!) Click it now!
182  * To start CI testing from this screen, you need to fill in two elements on
183    the form:
184    * The `CERTIFY_SAFE` box should be checked. By checking it, you are
185      indicating that you have reviewed the code you are about to test and you
186      are confident that it does not contain any malicious code. (We don't want
187      people hijacking our CI hosts to attack other hosts on the internet, for
188      example!)
189    * The `PR_ID` box should be filled in with the number identifying the pull
190      request containing the code you wish to test. For example, if the URL for
191      the pull request is `https://github.com/nodejs/node/issues/7006`, then put
192      `7006` in the `PR_ID`.
193    * The remaining elements on the form are typically unchanged.
194  * If you need help with something CI-related:
195    * Use the [Build WG repository](https://github.com/nodejs/build) to file
196      issues for the Build WG members who maintain the CI infrastructure.
197
198## Landing pull requests
199
200See the Collaborator Guide: [Landing pull requests][].
201
202Commits in one pull request that belong to one logical change should
203be squashed. It is rarely the case in onboarding exercises, so this
204needs to be pointed out separately during the onboarding.
205
206<!-- TODO(joyeechueng): provide examples about "one logical change" -->
207
208## Exercise: Make a pull request adding yourself to the README
209
210* Example:
211  <https://github.com/nodejs/node/commit/6669b3857f0f43ee0296eb7ac45086cd907b9e94>
212  * For raw commit message:
213    `git show --format=%B 6669b3857f0f43ee0296eb7ac45086cd907b9e94`
214* Collaborators are in alphabetical order by GitHub username.
215* Optionally, include your personal pronouns.
216* Add the `Fixes: <collaborator-nomination-issue-url>` to the commit message
217  so that when the commit lands, the nomination issue url will be
218  automatically closed.
219* Label your pull request with the `doc`, `notable-change`, and `fast-track`
220  labels. The `fast-track` label should cause the Node.js GitHub bot to post a
221  comment in the pull request asking collaborators to approve the pull request
222  by leaving a �� reaction on the comment.
223* Optional: Run CI on the pull request. Use the `node-test-pull-request` CI
224  task. As a convenience, you may apply the `request-ci` label to the pull
225  request to have a GitHub Actions workflow start the Jenkins CI task for you.
226* After two Collaborator approvals for the change and two Collaborator approvals
227  for fast-tracking, land the PR.
228* If there are not enough approvals within a reasonable time, consider the
229  single approval of the onboarding TSC member sufficient, and land the pull
230  request.
231  * Be sure to add the `PR-URL: <full-pr-url>` and appropriate `Reviewed-By:`
232    metadata.
233  * [`node-core-utils`][] automates the generation of metadata and the landing
234    process. See the documentation of [`git-node`][].
235  * [`core-validate-commit`][] automates the validation of commit messages.
236    This will be run during `git node land --final` of the [`git-node`][]
237    command.
238
239## Final notes
240
241* Don't worry about making mistakes: everybody makes them, there's a lot to
242  internalize and that takes time (and we recognize that!)
243* Almost any mistake you could make can be fixed or reverted.
244* The existing collaborators trust you and are grateful for your help!
245* Other repositories:
246  * <https://github.com/nodejs/TSC>
247  * <https://github.com/nodejs/build>
248  * <https://github.com/nodejs/nodejs.org>
249  * <https://github.com/nodejs/Release>
250  * <https://github.com/nodejs/citgm>
251* The OpenJS Foundation hosts regular summits for active contributors to the
252  Node.js project, where we have face-to-face discussions about our work on the
253  project. The Foundation has travel funds to cover participants' expenses
254  including accommodations, transportation, visa fees, etc. if needed. Check out
255  the [summit](https://github.com/nodejs/summit) repository for details.
256* If you are interested in helping to fix coverity reports consider requesting
257  access to the projects coverity project as outlined in [static-analysis][].
258
259[Code of Conduct]: https://github.com/nodejs/admin/blob/HEAD/CODE_OF_CONDUCT.md
260[Labels]: doc/contributing/collaborator-guide.md#labels
261[Landing pull requests]: doc/contributing/collaborator-guide.md#landing-pull-requests
262[Publicizing or hiding organization membership]: https://help.github.com/articles/publicizing-or-hiding-organization-membership/
263[`author-ready`]: doc/contributing/collaborator-guide.md#author-ready-pull-requests
264[`core-validate-commit`]: https://github.com/nodejs/core-validate-commit
265[`git-node`]: https://github.com/nodejs/node-core-utils/blob/HEAD/docs/git-node.md
266[`node-core-utils`]: https://github.com/nodejs/node-core-utils
267[set up the credentials]: https://github.com/nodejs/node-core-utils#setting-up-github-credentials
268[static-analysis]: doc/contributing/static-analysis.md
269[two-factor authentication]: https://help.github.com/articles/securing-your-account-with-two-factor-authentication-2fa/
270[using a TOTP mobile app]: https://help.github.com/articles/configuring-two-factor-authentication-via-a-totp-mobile-app/
271[who-to-cc]: doc/contributing/collaborator-guide.md#who-to-cc-in-the-issue-tracker
272