FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: align doc/api/tls.markdown with style guide and fix minor grammatical issues by svozza · Pull Request #5706 · nodejs/node · GitHub

/ node Public

doc: align doc/api/tls.markdown with style guide and fix minor grammatical issues - #5706

Closed
svozza wants to merge 1 commit into
nodejs:masterfrom
svozza:doc-tls-style-guide
ConversationCommits (1)ChecksFiles changed
Closed

doc: align doc/api/tls.markdown with style guide and fix minor grammatical issues#5706
svozza wants to merge 1 commit into
nodejs:masterfrom
svozza:doc-tls-style-guide

Conversation

svozza commented Mar 14, 2016

Copy link
Copy Markdown
Contributor

Affected core subsystem(s)

Doc, TLS

Description of change

As discussed in #5677 (and tangentially, nodejs/docs#87) I've broken out the extra changes originally made in that PR into a separate one. Brings tls.markdown into alignment with the Node.js styleguide, specifically regarding the use of personal pronouns. Also, fixes various typos, punctuation errors, line wrapping violations, missing definite/indefinite articles and other minor grammatical issues.

claudiorodriguez added tls Issues and PRs related to the tls subsystem. doc Issues and PRs related to the documentations. labels Mar 15, 2016
svozza force-pushed the doc-tls-style-guide branch 5 times, most recently from 5e04f4b to 1b2aab8 Compare March 15, 2016 09:48
benjamingr reviewed Mar 15, 2016
View reviewed changes
Comment thread doc/api/tls.markdown Outdated

benjamingr Mar 15, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Nit: wouldn't it be more correct to say it implements the duplex stream interface (singular)?

benjamingr commented Mar 15, 2016

Copy link
Copy Markdown
Member

Very nice contribution - I left some comments - when those are addressed LGTM.

svozza commented Mar 15, 2016

Copy link
Copy Markdown
Contributor Author

Cool. I've addressed all the comments bar the two I need clarification on. I'll push the changes together when they're all done.

svozza force-pushed the doc-tls-style-guide branch 2 times, most recently from d6c2422 to e87183c Compare March 15, 2016 12:49

svozza commented Mar 15, 2016

Copy link
Copy Markdown
Contributor Author

So I think I've addressed all the comments.

benjamingr commented Mar 15, 2016

Copy link
Copy Markdown
Member

Great, LGTM.

Allow me to clarify the process - pull requests are open for 48 hours during week days in order to give collaborators opportunity to respond to them and suggest changes. In a day and a half assuming no one has objections (and hopefully more people read this) this will be landed in master.

Pinging @nodejs/documentation in order to draw more attention to this issue since it's a big copy change.

svozza commented Mar 15, 2016

Copy link
Copy Markdown
Contributor Author

Good stuff. :)

svozza force-pushed the doc-tls-style-guide branch 3 times, most recently from 3a9d37a to cb8297f Compare March 15, 2016 13:57
doc: align doc/api/tls.markdown with style guide
f246ec3 
Brings tls.markdown into alignment with the node.js
styleguide, specifically regarding the use of
personal pronouns. Also, fixes various typos,
punctuation errors, missing definite/indefinite
articles and other minor grammatical issues.
svozza force-pushed the doc-tls-style-guide branch from cb8297f to f246ec3 Compare March 15, 2016 14:01

svozza commented Mar 15, 2016

Copy link
Copy Markdown
Contributor Author

Actually, something I noticed is that none of the anchor links in the file work when I view the markdown files in the browser. Take, for example, this one which is supposed to point to TLS.createServer():

Actual:   #tls_tls_createserver_options_secureconnectionlistener
Expected: #tlscreateserveroptions-secureconnectionlistener

jasnell commented Mar 16, 2016

Copy link
Copy Markdown
Member

LGTM

benjamingr pushed a commit that referenced this pull request Mar 20, 2016
doc: align doc/api/tls.markdown with style guide
4f6ad5c 
Brings tls.markdown into alignment with the node.js
styleguide, specifically regarding the use of
personal pronouns. Also, fixes various typos,
punctuation errors, missing definite/indefinite
articles and other minor grammatical issues.

PR-URL: #5706
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

benjamingr commented Mar 20, 2016

Copy link
Copy Markdown
Member

Landed in 4f6ad5c 🎈🎈🎈

Thanks again.

benjamingr closed this Mar 20, 2016

svozza commented Mar 20, 2016

Copy link
Copy Markdown
Contributor Author

👍

Fishrock123 mentioned this pull request Mar 22, 2016
Merged
Fishrock123 pushed a commit that referenced this pull request Mar 22, 2016
doc: align doc/api/tls.markdown with style guide
3ada8cc 
Brings tls.markdown into alignment with the node.js
styleguide, specifically regarding the use of
personal pronouns. Also, fixes various typos,
punctuation errors, missing definite/indefinite
articles and other minor grammatical issues.

PR-URL: #5706
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Conflicts:
	doc/api/tls.markdown

MylesBorins commented Mar 30, 2016

Copy link
Copy Markdown
Contributor

4f6ad5c is not landing cleanly onto v4.x-staging. Would someone be willing to backport?

svozza commented Mar 31, 2016

Copy link
Copy Markdown
Contributor Author

I can try, what exactly do I have to do? Just checkout the v4.x-staging branch and raise a PR against that?

MylesBorins commented Mar 31, 2016

Copy link
Copy Markdown
Contributor

@svozza check out v4.x-staging. then git cherry-pick 4f6ad5c. then work out the conflicts and send a PR to v4.x-staging

svozza commented Mar 31, 2016

Copy link
Copy Markdown
Contributor Author

Good stuff. I'll give that a go.

svozza mentioned this pull request Mar 31, 2016
Closed
svozza added a commit to svozza/node that referenced this pull request Apr 11, 2016
doc: align doc/api/tls.markdown with style guide
58716cc 
Brings tls.markdown into alignment with the node.js
styleguide, specifically regarding the use of
personal pronouns. Also, fixes various typos,
punctuation errors, missing definite/indefinite
articles and other minor grammatical issues.

PR-URL: nodejs#5706
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

doc Issues and PRs related to the documentations. tls Issues and PRs related to the tls subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL