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