Skip to content

Conversation

@ghost
Copy link

@ghostghost commented Dec 7, 2015

No description provided.

@mscdexmscdex added the doc Issues and PRs related to the documentations. label Dec 7, 2015
@Fishrock123
Copy link
Contributor

LGTM

README.md Outdated
Copy link
Member

Choose a reason for hiding this comment

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

nit: Node.js' package ecosystem -> The Node.js package ecosystem

@mscdex
Copy link
Contributor

Commit message should be prefixed with doc:.

@ghostghost changed the title Add explanatory text.doc: Add explanatory text.Dec 7, 2015
README.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you drop the apostrophe from The Node.js'.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please wrap lines at 80 characters.

@cjihrig
Copy link
Contributor

LGTM with comments.

@ghost
Copy link
Author

ghost commented Dec 7, 2015

👍

@JungMinu
Copy link
Member

would you please squash the commits? :)

@ghost
Copy link
Author

ghost commented Dec 7, 2015

Thank you.

@ghost
Copy link
Author

ghost commented Dec 8, 2015

With corrections made can we pull this in?

@Trott
Copy link
Member

Trott commented Dec 8, 2015

LGTM

@ghost
Copy link
Author

ghost commented Dec 8, 2015

I don't have merge privileges, so that may be the confusion.

Trott pushed a commit that referenced this pull request Dec 8, 2015
Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> PR-URL: #4174
@Trott
Copy link
Member

Trott commented Dec 8, 2015

Landed in a1388bb. Thanks. (There were some trailing spaces I trimmed, no big deal, but if you plan on doing more contributions in the future, FYI.)

@TrottTrott closed this Dec 8, 2015
@ghost ghost deleted the patch-1 branch December 8, 2015 23:52
@ghost
Copy link
Author

ghost commented Dec 8, 2015

Excellent. I'll watch for trailing spaces next time.

rvagg pushed a commit that referenced this pull request Dec 9, 2015
Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> PR-URL: #4174
@rvaggrvagg mentioned this pull request Dec 17, 2015
MylesBorins pushed a commit that referenced this pull request Dec 29, 2015
Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> PR-URL: #4174
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> PR-URL: #4174
@MylesBorinsMylesBorins mentioned this pull request Jan 19, 2016
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> PR-URL: nodejs#4174
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docIssues and PRs related to the documentations.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@Fishrock123@mscdex@cjihrig@JungMinu@Trott@jasnell@MylesBorins