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: use consistent typography for Note: #6987

Closed
wants to merge 2 commits into from
Closed

Conversation

Trott
Copy link
Member

@Trott Trott commented May 26, 2016

Checklist
  • the commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

Typography choices for Note: text is all over the map in stream.md.

This change introduces the following consistently for Note: text:

  • Only Note: is in bold.
  • Additional emphasis is done with italics, not CAPITALIZATION.
  • The first word after Note: is capitalized.
  • When classes are mentioned, they are enclosed in backticks.
  • Only one space is used between sentences.

/cc @nodejs/documentation

Typography choices for `Note:` text is all over the map in stream.md.

This change introduces the following consistently for `Note:` text:

* Only `Note:` is in bold.
* Additional emphasis is done with _italics_, not CAPITALIZATION.
* The first word after `Note:` is capitalized.
* When classes are mentioned, they are enclosed in backticks.
* Only one space is used between sentences.
@Trott Trott added the doc Issues and PRs related to the documentations. label May 26, 2016
@mscdex mscdex added the stream Issues and PRs related to the stream subsystem. label May 26, 2016
@thefourtheye
Copy link
Contributor

LGTM

@@ -924,8 +924,8 @@ becomes available. There is no need, for example to "wait" until
Buffer encoding, such as `'utf8'` or `'ascii'`
* return {Boolean} Whether or not more pushes should be performed

Note: **This method should be called by Readable implementors, NOT
by consumers of Readable streams.**
**Note:** This method should be called by Readable implementors, _not_
Copy link
Member

Choose a reason for hiding this comment

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

Readable should be backticked, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

@bengl Yup, it sure should. I'll update the PR.

@bengl
Copy link
Member

bengl commented May 26, 2016

Is there (could there be (should there be)) a tracking issue for cleaning this up in the rest of the docs?

@jasnell
Copy link
Member

jasnell commented May 26, 2016

I am working on normalizing the various *Note* formats used throughout the docs as part of my "general improvements" doc PRs with the intention of making a doctool update that will provide a consistent visual treatment for those across the board. The pattern that I've been using is *Note*: Text. The doctool I have in mind will look for the *Note* pattern and will replace those in the HTML with a proper CSS-styled <aside>.

@jasnell
Copy link
Member

jasnell commented May 27, 2016

oh... also, can we fold these edits up into the existing #6947?

@Trott
Copy link
Member Author

Trott commented May 27, 2016

@jasnell: I'm reading your first comment as indicating that I should change these to *Note*: rather than *Note:*. I'm happy to do that (and in fact, that's more correct in my mind, although some people find it visually jarring, but whatever, I just want it consistent).

As for #6947, I'm not sure what you mean by fold up. Which of these is the action you'd like to see taken? Either is fine with me. I'm just not sure which you mean.

Or did you mean something else?

I'm also fine with just leaving this open until #6947 lands and then inspecting the results to see if this PR should be rebased and modified to deal with any stray notes that were missed.

@Trott
Copy link
Member Author

Trott commented May 27, 2016

Is there (could there be (should there be)) a tracking issue for cleaning this up in the rest of the docs?

Sure, feel free to open such an issue. :-P

@benjamingr
Copy link
Member

LGTM

@jasnell
Copy link
Member

jasnell commented May 27, 2016

@Trott ... several of the edits in this PR are already handled in #6947 and the ones that aren't can easily be added. If you'd like to open a PR against my branch I'll pull it in.

@Trott Trott closed this Jun 6, 2016
@Trott Trott deleted the note branch January 13, 2022 22:43
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. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants