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: add ronkorving to collaborators #6385

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
5 participants
@ronkorving
Copy link
Contributor

commented Apr 26, 2016

Checklist
  • documentation is changed or added
  • the commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

Added myself as a collaborator following the onboarding process with @Trott

@ronkorving ronkorving added the doc label Apr 26, 2016

@evanlucas

This comment has been minimized.

Copy link
Member

commented Apr 26, 2016

Yay. LGTM

@jbergstroem

This comment has been minimized.

Copy link
Member

commented Apr 26, 2016

LGTM and welcome 🎉

@ronkorving

This comment has been minimized.

Copy link
Contributor Author

commented Apr 26, 2016

Thanks guys :)
Testing CI access through https://ci.nodejs.org/job/node-test-pull-request/2394/

@Trott

This comment has been minimized.

Copy link
Member

commented Apr 26, 2016

LGTM. Good to see you!

@ronkorving

This comment has been minimized.

Copy link
Contributor Author

commented Apr 26, 2016

Thanks a lot for your time and sharing your wisdom, @Trott :)

@Trott

This comment has been minimized.

Copy link
Member

commented Apr 26, 2016

(Before landing, can you confirm that the email address provided in this commit is the same one that are used as the author of your commits?)

@ronkorving

This comment has been minimized.

Copy link
Contributor Author

commented Apr 26, 2016

Is the same 👍

@estliberitas estliberitas force-pushed the nodejs:master branch 2 times, most recently to c7066fb Apr 26, 2016

ronkorving added a commit that referenced this pull request Apr 26, 2016

doc: add ronkorving to collaborators
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: #6385
@ronkorving

This comment has been minimized.

Copy link
Contributor Author

commented Apr 26, 2016

Landed in 916b1a1

@ronkorving ronkorving closed this Apr 26, 2016

@ronkorving ronkorving deleted the ronkorving:collaborator-update branch Apr 26, 2016

@jbergstroem

This comment has been minimized.

Copy link
Member

commented Apr 26, 2016

@ronkorving: next time, please put PR-URL on top of metadata. We don't have a strict ruleset for commit messages just now, but 99% of the commits follow at least that guideline.

@Trott

This comment has been minimized.

Copy link
Member

commented Apr 26, 2016

@jbergstroem Should we just make it official and add it to the docs, at least as a recommendation/preference?

@evanlucas

This comment has been minimized.

Copy link
Member

commented Apr 26, 2016

yes please!!

@jbergstroem

This comment has been minimized.

Copy link
Member

commented Apr 26, 2016

@Trott: I'm happy to put something together; that way I get what I want 😈

Trott added a commit to Trott/io.js that referenced this pull request Apr 26, 2016

doc: note that PR-URL should be first metadata
Convention has coalesced around putting the PR-URL as the first metadata
item in pull requests, so much so that collaborators are now flagging
other collaborators' commit messages when they don't do that. So let's
make it official and document that the PR-URL should be first.

Refs: nodejs/build#325
Refs: nodejs#6385

@Trott Trott referenced this pull request Apr 26, 2016

Closed

doc: note that PR-URL should be first metadata #6391

2 of 2 tasks complete
@ronkorving

This comment has been minimized.

Copy link
Contributor Author

commented Apr 26, 2016

Yeah, I indeed followed the onboarding doc's bullet list order :) Thanks for the PR @Trott, to make it a bit clearer.

jasnell added a commit that referenced this pull request Apr 26, 2016

doc: add ronkorving to collaborators
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: #6385

MylesBorins added a commit that referenced this pull request Jun 1, 2016

doc: add ronkorving to collaborators
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: #6385

@MylesBorins MylesBorins referenced this pull request Jun 24, 2016

Merged

v4.4.7 proposal #7395

MylesBorins added a commit that referenced this pull request Jun 24, 2016

doc: add ronkorving to collaborators
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: #6385

MylesBorins added a commit that referenced this pull request Jun 24, 2016

doc: add ronkorving to collaborators
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: #6385
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.