Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

doc: improve onboarding instructions #19108

Closed
wants to merge 2 commits into from
Closed

doc: improve onboarding instructions #19108

wants to merge 2 commits into from

Conversation

joyeecheung
Copy link
Member

  • Suggest to use a TOTP app if the new collaborator cannot receive
    SMS messages from GitHub
  • Suggest to install node-core-utils and set up credentials before
    the onboarding session.
  • Ask about subsystem teams that they want to join
  • Note about squashing commits
  • Other small fixes
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

@joyeecheung
doc: improve onboarding instructions
bd3a23d 
- Suggest to use a TOTP app if the new collaborator cannot receive
  SMS messages from GitHub
- Suggest to install node-core-utils and set up credentials before
  the onboarding session.
- Ask about subsystem teams that they want to join
- Note about squashing commits
- Other small fixes
@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Mar 3, 2018
richardlau
richardlau approved these changes Mar 3, 2018
View reviewed changes
COLLABORATOR_GUIDE.md Outdated
to type the password of your GitHub account in the console so the tool can
create the GitHub access token for you. If you do not want to do that, follow
[the guide of `node-core-utils`][node-core-utils-credentials]
to type the password of your GitHub account and the 2FA code in the console
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The onboarding doc writes out two-factor authentication and links to the GitHub help page.

@richardlau
Copy link
Member

As an aside, I just started using node-core-utils yesterday and it's really great ❤️ .

vsemozhetbyt
vsemozhetbyt reviewed Mar 3, 2018
View reviewed changes
doc/onboarding.md
* [`node-core-utils`][] automates the generation of metadata and the landing
process. See the documentation of [`git-node`][].
* [`core-validate-commit`][] automates the validation of commit messages.
This will be run during `git node land --final` of the [`git-node`][]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: it seems [`git-node`][] URL needs updating.

Trott
Trott reviewed Mar 3, 2018
View reviewed changes
doc/onboarding.md Outdated
* Announce the accepted nomination in a TSC meeting and in the TSC
mailing list.
* Suggest the new Collaborator to install [`node-core-utils`][] and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: remove to

gibfahn
gibfahn reviewed Mar 5, 2018
View reviewed changes
Copy link
Member

@gibfahn gibfahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nits

COLLABORATOR_GUIDE.md Outdated
@@ -471,9 +471,9 @@ $ git node land $PRID
```

If it's the first time you ever use `node-core-utils`, you will be prompted
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's not changed in this PR, but while you're here:

you ever use -> you have used

COLLABORATOR_GUIDE.md Outdated
[the guide of `node-core-utils`][node-core-utils-credentials]
to type the password of your GitHub account and the 2FA code in the console
so the tool can create the GitHub access token for you. If you do not want to
do that, follow [the guide of `node-core-utils`][node-core-utils-credentials]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the guide of node-core-utils -> the node-core-utils guide

doc/onboarding.md Outdated

## Fifteen minutes before the onboarding session

* Prior to the onboarding session, add the new Collaborator to
[the Collaborators team](https://github.com/orgs/nodejs/teams/collaborators).
* Ask them if they want to join any subsystem team. See
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

team -> teams

gibfahn
gibfahn approved these changes Mar 5, 2018
View reviewed changes
Copy link
Member

@gibfahn gibfahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nits

@joyeecheung
Copy link
Member Author

Fixed nits. Lite CI: https://ci.nodejs.org/job/node-test-pull-request-lite/233/

@joyeecheung
Copy link
Member Author

Landed in 42e9b48, thanks!

@joyeecheung joyeecheung closed this Mar 7, 2018
joyeecheung added a commit that referenced this pull request Mar 7, 2018
@joyeecheung
doc: improve onboarding instructions
42e9b48 
- Suggest to use a TOTP app if the new collaborator cannot receive
  SMS messages from GitHub
- Suggest to install node-core-utils and set up credentials before
  the onboarding session.
- Ask about subsystem teams that they want to join
- Note about squashing commits
- Other small fixes

PR-URL: #19108
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
targos pushed a commit that referenced this pull request Mar 17, 2018
@joyeecheung @targos
doc: improve onboarding instructions
9fe8646 
- Suggest to use a TOTP app if the new collaborator cannot receive
  SMS messages from GitHub
- Suggest to install node-core-utils and set up credentials before
  the onboarding session.
- Ask about subsystem teams that they want to join
- Note about squashing commits
- Other small fixes

PR-URL: #19108
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@targos targos mentioned this pull request Mar 18, 2018
Merged
MylesBorins pushed a commit that referenced this pull request Mar 20, 2018
@joyeecheung @MylesBorins
doc: improve onboarding instructions
27088cf 
- Suggest to use a TOTP app if the new collaborator cannot receive
  SMS messages from GitHub
- Suggest to install node-core-utils and set up credentials before
  the onboarding session.
- Ask about subsystem teams that they want to join
- Note about squashing commits
- Other small fixes

PR-URL: #19108
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
@joyeecheung @MayaLekova
doc: improve onboarding instructions
afd893b 
- Suggest to use a TOTP app if the new collaborator cannot receive
  SMS messages from GitHub
- Suggest to install node-core-utils and set up credentials before
  the onboarding session.
- Ask about subsystem teams that they want to join
- Note about squashing commits
- Other small fixes

PR-URL: nodejs#19108
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@jasnell
Copy link
Member

jasnell commented Aug 17, 2018

Doesn't need to be landed in 8.x or 6.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vsemozhetbyt vsemozhetbyt vsemozhetbyt left review comments

@Trott Trott Trott left review comments

@gibfahn gibfahn gibfahn approved these changes

@richardlau richardlau richardlau approved these changes

@lpinca lpinca lpinca approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@joyeecheung @richardlau @jasnell @Trott @lpinca @vsemozhetbyt @gibfahn @nodejs-github-bot