Skip to content

doc: use consistent markdown in README - #7971

Closed
Trott wants to merge 4 commits into
nodejs:masterfrom
Trott:remark
Closed

doc: use consistent markdown in README#7971
Trott wants to merge 4 commits into
nodejs:masterfrom
Trott:remark

Conversation

@Trott

@Trott Trott commented Aug 3, 2016

Copy link
Copy Markdown
Member
Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

Changes include:

  • consistent header indications
  • wrap bare URLs in < and >
  • consistent emphasis indications
  • specifying a language in fenced code
  • wrapping at 80 characters

Linting tool possibly on the way.

@Trott Trott added the doc Issues and PRs related to Node.js documentation. label Aug 3, 2016
Comment thread README.md Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Trott Trott Aug 4, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I disagree, but I'm open to be persuaded. Bike shed time, I guess but here are the reasons:

  • This is a shell command. It may be bash. Or it maybe csh or zsh or tcsh or even Windows PowerShell. There is nothing bash-specific about curl.
  • If shell is not more appropriate than bash here, then when would it ever be more appropriate?
  • This is more of an argument that it doesn't matter than that one or the other is right, but bash is an alias for shell in the syntax highlighting tool used by GitHub Flavored Markdown so there is no difference in the rendered output of one versus the other.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

¯_(ツ)_/¯

@ChALkeR ChALkeR Aug 4, 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.

This should be either «```console» or commands should not start with «$».

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

Improve markdown practices in README:

* consistent header indicators
* consistent emphasis indicators
* wrap bare URLs in `<` and `>`
* wrap at 80 chars
* specifying language in fenced code
Comment thread README.md Outdated

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.

ditto

@ChALkeR ChALkeR mentioned this pull request Aug 4, 2016
2 tasks
Comment thread README.md Outdated
The full set of trusted release keys can be imported by running:

```
```console

ghost Aug 4, 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.

This either shouldn't be «```console» or the lines should start with a prompt like «$ » or «> ».
Or this is going to be formatted like text output.

I would just use sh or shell here (in this specific block), perhaps.

@ChALkeR

ghost commented Aug 4, 2016

Copy link
Copy Markdown
Member

One issue with a code block language specification (consoleshell), everything else LGTM.

@Trott

ghost commented Aug 4, 2016

Copy link
Copy Markdown
Member Author

@ChALkeR Switched that one instance to shell. Thanks.

@ChALkeR

ghost commented Aug 4, 2016

Copy link
Copy Markdown
Member

@Trott Ah, noticed one more thing.

$ gpg --keyserver pool.sks-keyservers.net \
  --recv-keys DD8F2338BAE7501E3DD5AC78C273792F7D83545D

is inaccurate and gets wrong highlight.

It should be either shell without the $ to allow copy-pasting the whole thing:

gpg --keyserver pool.sks-keyservers.net \
  --recv-keys DD8F2338BAE7501E3DD5AC78C273792F7D83545D

or console with a > on the second line to depicture the actual console behaviour and serve as an example:

$ gpg --keyserver pool.sks-keyservers.net \
>  --recv-keys DD8F2338BAE7501E3DD5AC78C273792F7D83545D

@Trott

ghost commented Aug 4, 2016

Copy link
Copy Markdown
Member Author

@ChALkeR I suppose another option is just to get rid of the line break and have it be a single line by itself. Does that work for you?

@ChALkeR

ghost commented Aug 4, 2016

Copy link
Copy Markdown
Member

@Trott Yes, it does, it doesn't break the maximum-line-length rule.

It does break a guideline rule regarding the sources code style, but there is no other way to achieve a similar display and this looks like a valid reason for an exception. We already have those in other places (long links, for example).

LGTM.

@targos

ghost commented Aug 4, 2016

Copy link
Copy Markdown
Member

LGTM

@jasnell

ghost commented Aug 4, 2016

Copy link
Copy Markdown
Member

LGTM!

@jasnell

ghost commented Aug 5, 2016

Copy link
Copy Markdown
Member

Landed in cc3a9e7!

@jasnell jasnell closed this Aug 5, 2016
@cjihrig cjihrig mentioned this pull request Aug 8, 2016
@cjihrig cjihrig mentioned this pull request Aug 11, 2016
@MylesBorins MylesBorins mentioned this pull request Oct 26, 2016
@Trott
Trott deleted the remark branch September 17, 2021 18:21
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 Node.js documentation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants