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.

Sign up for GitHub

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

Jump to bottom

Doc: remove non-standard use of hyphens in doc/api/tls.markdown #5677

Closed
wants to merge 1 commit into from
Closed

Doc: remove non-standard use of hyphens in doc/api/tls.markdown #5677

wants to merge 1 commit into from

Conversation

svozza
Copy link
Contributor

@svozza svozza commented Mar 12, 2016

Affected core subsystem(s)

Doc, TLS

Description of change

Fixes #5672. Removes the non-idiomatic usages of the '-' character. I also tidied up some of the phrasing, grammar, typos and punctuation throughout the whole file. If I should have done this in a separate PR, I'm happy to pull them out and leave only the hyphen changes.

Also, there seems to be an inconsistent use of 'can', 'could' and 'may' throughout the document but I decided against changing that as I don't really know what the house style is.

@claudiorodriguez claudiorodriguez added tls Issues and PRs related to the tls subsystem. doc Issues and PRs related to the documentations. labels Mar 12, 2016
@claudiorodriguez
Copy link
Contributor

LGTM. I think it might be a good idea to clarify this on the style guide (https://github.com/nodejs/docs/blob/master/STYLE-GUIDE.md), gonna make an issue there.

@claudiorodriguez claudiorodriguez mentioned this pull request Mar 12, 2016
Closed
@svozza
Copy link
Contributor Author

svozza commented Mar 12, 2016

Ah, I didn't see the style guide, very handy. I've just updated the PR to remove all occurrences of the word 'you' in light of that document.

@eljefedelrodeodeljefe
Copy link
Contributor

This PR has more than just the described changes in it. Can you exclude them please? Also the use of youshould be discussed in broader scope in the docs WG.

@svozza svozza force-pushed the docs-tls-punctuation branch 2 times, most recently from 9c062f5 to 202b99f Compare March 12, 2016 22:16
@svozza
Copy link
Contributor Author

svozza commented Mar 12, 2016

Sure thing. Can I open a separate PR with the other changes? I think the document reads much better than before.

@eljefedelrodeodeljefe
Copy link
Contributor

Sure, generally. Just two remarks here. Not every hyphen qualifies for removal. Those behind acronyms especially, since those seem quite consistent to the rest of the docs. If there is something wrong in general in english or style, I'd rather suggest opening an issue at nodejs/docs. Second, while I agree on the hyphens seemingly being misplaced and hence having inferior readability of the doc, you probably have to make a good case for the other changes. But please, PR is always welcome. We discuss it there.

@svozza
Copy link
Contributor Author

svozza commented Mar 12, 2016

Those behind acronyms especially, since those seem quite consistent to the rest of the docs.

Do you mean 'in front of'? E.g., the ALPN/NPN and SNI ones?

Aside from the personal pronoun stuff, which I will omit from the PR, I think the other changes are pretty uncontroversial, they're mainly just fixing typos and missing definite/indefinite articles.

@eljefedelrodeodeljefe
Copy link
Contributor

Yes, I mean those.

Sure, they are. They are also a bit of a nit, the WG might want to have in a broader effort. Those kind of problems are all over the docs, I mean. We'll see...

@jasnell
Copy link
Member

jasnell commented Mar 12, 2016

LGTM

@svozza
Copy link
Contributor Author

svozza commented Mar 12, 2016

Yes, I mean those.

Cool. I'll change back those other hyphens.

@Trott
Copy link
Member

Trott commented Mar 13, 2016

Nit: Can you add Fixes: https://github.com/nodejs/node/issues/5672 to the bottom of the commit message? And maybe leave a comment on that issue so that no one else tries to grab it as a good first contribution?

@svozza
Copy link
Contributor Author

svozza commented Mar 13, 2016

No problem, all done.

@eljefedelrodeodeljefe
Copy link
Contributor

LGTM

@svozza svozza mentioned this pull request Mar 13, 2016
Closed
@svozza
doc: remove non-standard use of hyphens
9ad2ebd 
Identifies the non-idiomatic usages of the '-' character
and either removes them or replaces them with colons.

Fixes: nodejs#5672
jasnell pushed a commit that referenced this pull request Mar 14, 2016
@svozza @jasnell
doc: remove non-standard use of hyphens
ecbb955 
Identifies the non-idiomatic usages of the '-' character
and either removes them or replaces them with colons.

Fixes: #5672
R-URL: #5677
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
@jasnell
Copy link
Member

jasnell commented Mar 14, 2016

Landed in ecbb955

@jasnell jasnell closed this Mar 14, 2016
evanlucas pushed a commit that referenced this pull request Mar 14, 2016
@svozza @evanlucas
doc: remove non-standard use of hyphens
2a3394f 
Identifies the non-idiomatic usages of the '-' character
and either removes them or replaces them with colons.

Fixes: #5672
R-URL: #5677
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
@svozza svozza mentioned this pull request Mar 14, 2016
Closed
rvagg pushed a commit that referenced this pull request Mar 16, 2016
@svozza @rvagg
doc: remove non-standard use of hyphens
31e39fb 
Identifies the non-idiomatic usages of the '-' character
and either removes them or replaces them with colons.

Fixes: #5672
R-URL: #5677
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
MylesBorins pushed a commit that referenced this pull request Mar 17, 2016
@svozza
doc: remove non-standard use of hyphens
e4817bb 
Identifies the non-idiomatic usages of the '-' character
and either removes them or replaces them with colons.

Fixes: #5672
R-URL: #5677
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
@svozza
doc: remove non-standard use of hyphens
d8eeca4 
Identifies the non-idiomatic usages of the '-' character
and either removes them or replaces them with colons.

Fixes: #5672
R-URL: #5677
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
@svozza
doc: remove non-standard use of hyphens
c6dc561 
Identifies the non-idiomatic usages of the '-' character
and either removes them or replaces them with colons.

Fixes: #5672
R-URL: #5677
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
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.
6 participants
@svozza @claudiorodriguez @eljefedelrodeodeljefe @jasnell @Trott @MylesBorins