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.

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

doc: update ONBOARDING.md #1363

Merged
merged 9 commits into from
Jul 31, 2018
Merged

doc: update ONBOARDING.md #1363

merged 9 commits into from
Jul 31, 2018

Conversation

maclover7
Copy link
Contributor

Adds more concrete steps to the onboarding guide for the WG. This is
updated with the information I used to onboard @mmarchini.

Adds more concrete steps to the onboarding guide for the WG. This is
updated with the information I used to onboard @mmarchini.
@maclover7 maclover7 added the doc label Jun 18, 2018
@maclover7 maclover7 requested review from Trott and mmarchini June 18, 2018 23:52
Trott

This comment was marked as off-topic.

Trott

This comment was marked as off-topic.

Trott

This comment was marked as off-topic.

mmarchini

This comment was marked as off-topic.

@maclover7
Copy link
Contributor Author

updated @Trott @mmarchini

@maclover7
Copy link
Contributor Author

cc @LucaLanziani, anything else that should be added here that we went over?

joyeecheung

This comment was marked as off-topic.

LucaLanziani

This comment was marked as off-topic.

@maclover7
Copy link
Contributor Author

updated @LucaLanziani

LucaLanziani

This comment was marked as off-topic.

mhdawson

This comment was marked as off-topic.

mhdawson

This comment was marked as off-topic.

mhdawson

This comment was marked as off-topic.

mhdawson

This comment was marked as off-topic.

joaocgreis

This comment was marked as off-topic.

@maclover7
Copy link
Contributor Author

updated @mhdawson @joaocgreis, PTAL

gdams

This comment was marked as off-topic.

mhdawson

This comment was marked as off-topic.

@maclover7
Copy link
Contributor Author

Since this has 6 approvals and has been open for about a week, will try and land this tomorrow assuming no other comments

rvagg

This comment was marked as off-topic.

joaocgreis

This comment was marked as off-topic.

@maclover7
Copy link
Contributor Author

updated @rvagg @mhdawson PTAL

@rvagg
Copy link
Member

rvagg commented Jun 27, 2018

The section on pipelines is pretty important, there is definitely not consensus here that pipelines are a good idea as a "general trend". A couple of us have been highlighting how they can decrease usability for collaborators who just want to see test failure details. I'd rather than either not be included here or reworded to not make it sound like you should just assume pipelines are the way to go for everything.

@maclover7
Copy link
Contributor Author

Updated to remove the pipeline/bash section from the guide

@maclover7
Copy link
Contributor Author

ping @rvagg

1 similar comment
@maclover7
Copy link
Contributor Author

ping @rvagg

@rvagg
Copy link
Member

rvagg commented Jul 31, 2018

whoops! sorry @maclover7, +1 lgtm

@maclover7 maclover7 merged commit 4c5c10b into nodejs:master Jul 31, 2018
@maclover7
Copy link
Contributor Author

Landed in 4c5c10b -- thank you everybody for the review!

@maclover7 maclover7 deleted the jm-onboarding branch July 31, 2018 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants