From 727c24f3a22ee0159b0066405dda526d46187c58 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 12 Aug 2016 14:34:11 -0700 Subject: [PATCH] doc: update Reviewing section of onboarding doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL; https://github.com/nodejs/node/pull/8086 Reviewed-By: Anna Henningsen Reviewed-By: targos - Michaƫl Zasso Reviewed-By: Franziska Hinkelmann Reviewed-By: Evan Lucas Reviewed-By: jasnell - James M Snell --- doc/onboarding.md | 56 ++++++++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/doc/onboarding.md b/doc/onboarding.md index a44d2170bca544..991809df1ea40a 100644 --- a/doc/onboarding.md +++ b/doc/onboarding.md @@ -64,31 +64,37 @@ onboarding session. * 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