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

add missing ) in CONTRIBUTING.md #12444

Closed
wants to merge 1 commit into from
Closed

add missing ) in CONTRIBUTING.md #12444

wants to merge 1 commit into from

Conversation

matkoniecz
Copy link
Contributor

Checklist
Affected core subsystem(s)

doc

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Apr 16, 2017
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Can you prefix your commit message with doc:? Otherwise this looks good. 👍

@watilde
Copy link
Member

watilde commented Apr 17, 2017

I'm going to ship this to master.

@matkoniecz
Copy link
Contributor Author

@addaleax

Can you prefix your commit message with doc:? Otherwise this looks good. 👍

Done.

watilde pushed a commit to watilde/node that referenced this pull request Apr 17, 2017
PR-URL: nodejs#12444
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@watilde
Copy link
Member

watilde commented Apr 17, 2017

Landed in 6724f78. Thanks!

@watilde watilde closed this Apr 17, 2017
@watilde
Copy link
Member

watilde commented Apr 17, 2017

Oops, I didn't wait for 24h for landing, but I hope this change didn't need to wait for a day to land since it just fixed a typo in a doc.

Either way, I will check the created date of the PR carefully for next time. Thanks.

@vsemozhetbyt
Copy link
Contributor

@watilde It seems it is probably OK: #12221 (comment)

@matkoniecz
Copy link
Contributor Author

matkoniecz commented Apr 17, 2017

@watilde According to https://github.com/nodejs/node/blob/1e12c7396809e67de738b888d813301fb29f9efa/doc/onboarding.md#reviewing-prs There is a minimum waiting time which we try to respect for non-trivial changes" (bolding mine).

@jasnell
Copy link
Member

jasnell commented Apr 17, 2017

This one was more than ok to land inside of the 48 hour period.

@matkoniecz matkoniecz deleted the missing_closing_) branch April 18, 2017 04:00
evanlucas pushed a commit that referenced this pull request Apr 25, 2017
PR-URL: #12444
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@evanlucas evanlucas mentioned this pull request May 1, 2017
evanlucas pushed a commit that referenced this pull request May 1, 2017
PR-URL: #12444
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
evanlucas pushed a commit that referenced this pull request May 2, 2017
PR-URL: #12444
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
gibfahn pushed a commit that referenced this pull request May 15, 2017
PR-URL: #12444
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 18, 2017
PR-URL: #12444
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@MylesBorins MylesBorins mentioned this pull request May 23, 2017
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
PR-URL: nodejs/node#12444
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants