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: correct grammar in README.md #8892

Closed
wants to merge 2 commits into from
Closed

doc: correct grammar in README.md #8892

wants to merge 2 commits into from

Conversation

NoahDragon
Copy link
Contributor

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

Fix some punctuations.

Fix some punctuations.
@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Oct 2, 2016
@@ -44,7 +44,7 @@ The Node.js project maintains multiple types of releases:
versioned by [SemVer](http://semver.org/) and signed by a member of the
[Release Team](#release-team).
Code for Current releases is organized in this repository by major version
number, For example: [v4.x](https://github.com/nodejs/node/tree/v4.x).
number, For example, [v4.x](https://github.com/nodejs/node/tree/v4.x).
Copy link
Contributor

Choose a reason for hiding this comment

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

I think changing the preceding comma to a period would be a better change here:

number. For example: ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mscdex Agree.

@@ -73,7 +73,7 @@ Binaries, installers, and source tarballs are available at
<https://nodejs.org/download/release/>, listed under their version strings.
The [latest](https://nodejs.org/download/release/latest/) directory is an
alias for the latest Current release. The latest LTS release from an LTS
line is available in the form: latest-_codename_. For example:
line is available in the form: latest-_codename_. For example,
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO colon looks better here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mscdex Agree. I will make the change.

@mscdex
Copy link
Contributor

mscdex commented Oct 2, 2016

LGTM

Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

LGTM

@NoahDragon NoahDragon changed the title Correct grammar in README.md doc: correct grammar in README.md Oct 3, 2016
@NoahDragon
Copy link
Contributor Author

Just wonder, when normally the PR will be merged after review?

@gibfahn
Copy link
Member

gibfahn commented Oct 4, 2016

@NoahDragon The minimum is 48 hours, but we often wait a little longer. This has probably been long enough, so it's just a matter of a contributor having some time and merging it in. Should be soon though!

@gibfahn
Copy link
Member

gibfahn commented Oct 4, 2016

CI run: https://ci.nodejs.org/job/node-test-pull-request/4377/
(This is only relevant for the linter results)

@NoahDragon
Copy link
Contributor Author

@gibfahn Thanks for the clarification.

@gibfahn
Copy link
Member

gibfahn commented Oct 4, 2016

I'm seeing two linter failures, which are caused by #8873.

I'll start merging this now, I think it's been long enough for a doc change.

  • 2 LGTMs
  • No objections
  • No issues from make lint on my machine
$ make lint              
./node tools/eslint/bin/eslint.js --cache --rulesdir=tools/eslint-rules \
      benchmark lib test tools

/Users/gib/dev/node/lib/_stream_readable.js
  664:30  error  'i' is constant         no-const-assign
  670:9   error  'i' is already defined  no-redeclare

✖ 2 problems (2 errors, 0 warnings)

make: *** [jslint] Error 1

@MylesBorins
Copy link
Contributor

master is fixed, try again

gibfahn pushed a commit that referenced this pull request Oct 4, 2016
PR-URL: #8892
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
@gibfahn
Copy link
Member

gibfahn commented Oct 4, 2016

Linter failures resolved, linter now passes cleanly.

Landed in e10516d, thanks a lot @NoahDragon.

BTW @NoahDragon you changed the title of the PR, but not the name of the commit. In general the commit should start with subsystem: description of PR. I fixed it for you while merging it in.

@gibfahn gibfahn closed this Oct 4, 2016
@NoahDragon
Copy link
Contributor Author

@gibfahn Thank you.

@NoahDragon NoahDragon deleted the patch-1 branch October 4, 2016 17:09
@NoahDragon
Copy link
Contributor Author

How to become a Nodejs Foundation Member, I saw the website saying that anyone has contributed to Nodejs projects can grant free membership (Ref).

I know that I only did a really little contribution based on this PR, but do I qualified to become a Nodejs Foundation member on Github, or I still need to contribute more?

I will continue contributing Nodejs community because I'm maintaining a static generator (Hexo) which heavily relies on nodejs functionalities.

I don't know where to ask this question, and don't want to mess up the issue board. So I asked it here. Please forgive me if I posted it at wrong place.

@Fishrock123
Copy link
Contributor

cc @mikeal for details

generally I think if you get added to the org (you've done enough to become part of a team, working group, or a core collaborator), you have access to it. It may be removed in the future if you are inactive iirc.

You're best shot is to consistently contribute. ;)

@NoahDragon
Copy link
Contributor Author

Thank you @Fishrock123

jasnell pushed a commit that referenced this pull request Oct 6, 2016
PR-URL: #8892
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Fishrock123 pushed a commit that referenced this pull request Oct 11, 2016
PR-URL: #8892
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
@NoahDragon
Copy link
Contributor Author

Correct me if I'm wrong, based on the website (https://nodejs.org/en/foundation/members/#becoming-an-individual-member), I think any contributor could become a Nodejs Foundation member. Do I misunderstand it or there are some conditions on it?

Any reply are appreciated.

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.

7 participants