From 8f8db447dfbb87061eeb73d6fa252fb7d722a629 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Mon, 9 Nov 2015 17:52:43 -0500 Subject: [PATCH] doc: add onboarding resources PR-URL: https://github.com/nodejs/node/pull/3726 Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Chris Dickinson Reviewed-By: Rich Trott --- doc/onboarding-extras.md | 105 +++++++++++++++++++++ doc/onboarding.md | 199 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 304 insertions(+) create mode 100644 doc/onboarding-extras.md create mode 100644 doc/onboarding.md diff --git a/doc/onboarding-extras.md b/doc/onboarding-extras.md new file mode 100644 index 00000000000000..2f35e2778c3a2c --- /dev/null +++ b/doc/onboarding-extras.md @@ -0,0 +1,105 @@ +## Who to CC in issues + +* `lib/buffer`: @trevnorris +* `lib/child_process`: @cjihrig, @bnoordhuis, @piscisaereus +* `lib/cluster`: @cjihrig, @bnoordhuis, @piscisaereus +* `lib/{crypto,tls,https}`: @indutny, @shigeki, @nodejs/crypto +* `lib/domains`: @misterdjules +* `lib/{_}http{*}`: @indutny, @bnoordhuis, @nodejs/http +* `lib/net`: @indutny, @bnoordhuis, @piscisaereus, @chrisdickinson, @nodejs/streams +* `lib/{_}stream{s|*}`: @nodejs/streams +* `lib/repl`: @fishrock123 +* `lib/timers`: @fishrock123, @misterdjules +* `lib/zlib`: @indutny, @bnoordhuis + +* `src/async-wrap.*`: @trevnorris +* `src/node_crypto.*`: @indutny, @shigeki, @nodejs/crypto + +* `test/*`: @nodejs/testing, @trott + +* `tools/eslint`, `.eslintrc`: @silverwind, @trott + +* upgrading v8: @bnoordhuis / @targos / @ofrobots +* upgrading npm: @thealphanerd, @fishrock123 + + +When things need extra attention, are controversial, or `semver-major`: @nodejs/ctc + +If you cannot find who to cc for a file, `git shortlog -n -s ` may help. + + +## Labels + +### By Subsystem + +We generally sort issues by a concept of "subsystem" so that we know what part(s) of the codebase it touches. + +**Subsystems generally are**: + +* `lib/*.js` +* `doc`, `build`, `tools`, `test`, `deps`, `lib / src` (special), and there may be others. +* `meta` for anything non-code (process) related + +There may be more than one subsystem valid for any particular issue / PR. + + +### General + +Please use these when possible / appropriate + +* `confirmed-bug` - Bugs you have verified exist +* `discuss` - Things that need larger discussion +* `feature request` - Any issue that requests a new feature (usually not PRs) +* `good first contribution` - Issues suitable for newcomers to process + +* `semver-{minor,major}` + * be conservative – that is, if a change has the remote *chance* of breaking something, go for semver-major + * when adding a semver label, add a comment explaining why you're adding it + * minor vs. patch: roughly: "does it add a new method / does it add a new section to the docs" + * major vs. everything else: run last versions tests against this version, if they pass, **probably** minor or patch + * A breaking change helper ([full source](https://gist.github.com/chrisdickinson/ba532fa0e4e243fb7b44)): + ``` + git checkout $(git show -s --pretty='%T' $(git show-ref -d $(git describe --abbrev=0) | tail -n1 | awk '{print $1}')) -- test; make -j8 test + ``` + + +### Other Labels + +* Operating system labels + * `os x`, `windows`, `solaris` + * No linux, linux is the implied default +* Architecture labels + * `arm`, `mips` + * No x86{_64}, since that is the implied default +* `lts-agenda`, `lts-watch-v*` + * tag things that should be discussed to go into LTS or should go into a specific LTS branch + * (usually only semver-patch things) + * will come more naturally over time + + +## Updating Node.js from Upstream + +* `git remote add upstream git://github.com/nodejs/node.git` + +to update from nodejs/node: +* `git checkout master` +* `git remote update -p` OR `git fetch --all` (I prefer the former) +* `git merge --ff-only upstream/master` (or `REMOTENAME/BRANCH`) + + +## If `git am` fails + +* if `git am` fails – use `git am --abort` + * this usually means the PR needs updated + * prefer to make the originating user update the code, since they have it fresh in mind +* first, reattempt with `git am -3` (3-way merge)` +* if `-3` still fails, and you need to get it merged: + * `git fetch origin pull/N/head:pr-N && git checkout pr-N && git rebase master` + + +## best practices + +* commit often, out to your github fork (origin), open a PR +* when making PRs make sure to spend time on the description: + * every moment you spend writing a good description quarters the amount of time it takes to understand your code. +* usually prefer to only squash at the *end* of your work, depends on the change diff --git a/doc/onboarding.md b/doc/onboarding.md new file mode 100644 index 00000000000000..1646080f3b3bbf --- /dev/null +++ b/doc/onboarding.md @@ -0,0 +1,199 @@ +## pre-setup + +Ensure everyone is added to https://github.com/orgs/nodejs/teams/collaborators + + +## onboarding to nodejs + +### intros + + +### **thank you** for doing this + + * going to cover four things: + * local setup + * some project goals & values + * issues, labels, and reviewing code + * merging code + + +### setup: + + * notifications setup + * use https://github.com/notifications or set up email + * watching the main repo will flood your inbox, so be prepared + + + * git: + * make sure you have whitespace=fix: `git config --global --add core.whitespace fix` + * usually PR from your own github fork + * [**See "Updating Node.js from Upstream"**](./onboarding-extras.md#updating-nodejs-from-upstream) + * make new branches for all commits you make! + + + * `#node-dev` on `chat.freenode.net` is the best place to interact with the CTC / other collaborators + + +### a little deeper about the project + + * collaborators are effectively part owners + * the project has the goals of its contributors + + + * but, there are some higher-level goals and values + * not everything belongs in core (if it can be done reasonably in userland, let it stay in userland) + * empathy towards users matters (this is in part why we onboard people) + * generally: try to be nice to people + + +### managing the issue tracker + + * you have (mostly) free rein – don't hesitate to close an issue if you are confident that it should be closed + * this will come more naturally over time + * IMPORTANT: be nice about closing issues, let people know why, and that issues and PRs can be reopened if necessary + * Still need to follow the Code of Conduct. + + + * labels: + * generally sort issues by a concept of "subsystem" so that we know what part(s) of the codebase it touches, though there are also other useful labels. + * [**See "Labels"**](./onboarding-extras.md#labels) + * `ctc-agenda` if a topic is controversial or isn't coming to a conclusion after an extended time. + * `semver-{minor,major}`: + * be conservative – that is, if a change has the remote *chance* of breaking something, go for `semver-major` + * when adding a semver label, add a comment explaining why you're adding it + * it's cached locally in your brain at that moment! + + + * Notifying humans + * [**See "Who to CC in issues"**](./onboarding-extras.md#who-to-cc-in-issues) + * will also come more naturally over time + + + * reviewing: + * primary goal is for the codebase to improve + * secondary (but not far off) is for the person submitting code to succeed + * helps grow the community + * and draws new people into the project + * Review a bit at a time. It is **very important** to not overwhelm newer people. + * it is tempting to micro-optimize / make everything about relative perf, + don't succumb to that temptation. we change v8 a lot more often now, contortions + that are zippy today may be unnecessary in the future + * be aware: your opinion carries a lot of weight! + * nits are fine, but try to avoid stalling the PR + * note that they are nits when you comment + * if they really are stalling nits, fix them yourself on merge (but try to let PR authors know they can fix these) + * improvement doesn't have to come all at once + * minimum wait for comments time + * There is a minimum waiting time which we try to respect for non-trivial changes, so that people who may have important input in such a distributed project are able to respond. + * It may help to set time limits and expectations: + * the collaborators are very distributed so it is unlikely that they will be looking at stuff the same time as you are. + * before merging code: give folks at least one working day to respond: "If no one objects, tomorrow at