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: minor doc improvements #17722

Closed
wants to merge 3 commits into from
Closed

doc: minor doc improvements #17722

wants to merge 3 commits into from

Conversation

BridgeAR
Copy link
Member

Seems like some parts of the docs are currently actually not linted? Otherwise these files would probably result in a couple of errors. I went ahead and fixed things as I think is best. This does not mean they will already work with our lint rules and they might need further work but I think it is a good step into the right direction.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Dec 17, 2017
maclover7
maclover7 approved these changes Dec 18, 2017
View reviewed changes
Copy link
Contributor

@maclover7 maclover7 left a comment

Choose a reason for hiding this comment

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

In general, LGTM, although I'm a little concerned about the amount of git churn here. People obviously don't bisect the docs as much as actual code, but still useful to have a good history to refer back to.

doc/guides/building-node-with-ninja.md Outdated
1. Run `ninja -C out/Release` to produce a compiled release binary.
1. Lastly, make symlink to `./node` using `ln -fs out/Release/node node`.

When running `ninja -C out/Release` you will see output similar to the following
Copy link
Contributor

Choose a reason for hiding this comment

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

While we're here, can you add a comma after ninja -C out/Release

Copy link
Member Author

Choose a reason for hiding this comment

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

I would rather keep this PR only about stylistic changes.

@BridgeAR
Copy link
Member Author

I rebase tomorrow

CI https://ci.nodejs.org/job/node-test-commit-light/40/

richardlau
richardlau reviewed Dec 19, 2017
View reviewed changes
doc/guides/backporting-to-release-lines.md Outdated
2. Make sure that the local staging branch is up to date with the remote
3. Create a new branch off of the staging branch
1. Make sure that the local staging branch is up to date with the remote
1. Create a new branch off of the staging branch
Copy link
Member

@richardlau richardlau Dec 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I prefer the numbering of the steps before this change (i.e. When it didn't reset to 1 after the examples).

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I checked and most files use 1. 2. 3... even though there is at least one file that uses 1. 1. 1.... I personally prefer the latter as it allows to reorder things easily / removing a single entry without creating any churn.

Copy link
Member

Choose a reason for hiding this comment

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

My point is that this change has renumbered the steps so that instead of steps 1-10 it now reads steps 1-3, 1, 1-6.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right. I did not check that anymore. I just removed the commit as our style currently does not really rely on it. I guess it might be introduced as a general thing or it should not be used further at all.

@BridgeAR
Copy link
Member Author

Rebased and I removed a commit about changing the numbers.

@BridgeAR
Copy link
Member Author

CI https://ci.nodejs.org/job/node-test-commit-light/44/

@richardlau
Copy link
Member

Commit message nit: asterixs->asterisks

@BridgeAR
doc: update doc formatting to fit our 80 chars rule.
06b0326 
This will also use a proper indentation as a couple of entries
had a extra indentation of two spaces.
@BridgeAR
doc: use consistent new lines
f6810ac
@BridgeAR
doc: use dashes instead of asterisks
eef95c3
@BridgeAR
Copy link
Member Author

Rebased

@BridgeAR
Copy link
Member Author

New CI https://ci.nodejs.org/job/node-test-commit-light/51/

@BridgeAR
Copy link
Member Author

Landed in 9ca4ab1, 6924dac, 5c29013

@BridgeAR BridgeAR closed this Dec 26, 2017
BridgeAR added a commit to BridgeAR/node that referenced this pull request Dec 26, 2017
@BridgeAR
doc: update formatting to fit our 80 chars rule
5c29013 
This will also use a proper indentation as a couple of entries
had a extra indentation of two spaces.

PR-URL: nodejs#17722
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Dec 26, 2017
@BridgeAR
doc: use consistent new lines
6924dac 
PR-URL: nodejs#17722
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Dec 26, 2017
@BridgeAR
doc: use dashes instead of asterisks
9ca4ab1 
PR-URL: nodejs#17722
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 8, 2018
@BridgeAR @MylesBorins
doc: update formatting to fit our 80 chars rule
5e098a7 
This will also use a proper indentation as a couple of entries
had a extra indentation of two spaces.

PR-URL: #17722
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 8, 2018
@BridgeAR @MylesBorins
doc: use consistent new lines
317426a 
PR-URL: #17722
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 8, 2018
@BridgeAR @MylesBorins
doc: use dashes instead of asterisks
b89a02e 
PR-URL: #17722
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
@BridgeAR @MylesBorins
doc: update formatting to fit our 80 chars rule
d95408d 
This will also use a proper indentation as a couple of entries
had a extra indentation of two spaces.

PR-URL: #17722
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
@BridgeAR @MylesBorins
doc: use consistent new lines
081e701 
PR-URL: #17722
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
@BridgeAR @MylesBorins
doc: use dashes instead of asterisks
beab096 
PR-URL: #17722
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
@BridgeAR @MylesBorins
doc: update formatting to fit our 80 chars rule
a63d3c5 
This will also use a proper indentation as a couple of entries
had a extra indentation of two spaces.

PR-URL: #17722
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
@BridgeAR @MylesBorins
doc: use consistent new lines
26fbb0f 
PR-URL: #17722
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
@BridgeAR @MylesBorins
doc: use dashes instead of asterisks
1be0086 
PR-URL: #17722
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 10, 2018
Merged
gibfahn pushed a commit that referenced this pull request Jan 24, 2018
@BridgeAR @gibfahn
doc: use dashes instead of asterisks
838e5cc 
PR-URL: #17722
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
gibfahn pushed a commit that referenced this pull request Jan 24, 2018
@BridgeAR @gibfahn
doc: use dashes instead of asterisks
e6298c5 
PR-URL: #17722
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
@gibfahn
Copy link
Member

gibfahn commented Jan 24, 2018

Landed the first commit on 6.x and 8.x (the others conflicted).

gibfahn pushed a commit that referenced this pull request Jan 24, 2018
@BridgeAR @gibfahn
doc: use dashes instead of asterisks
8076127 
PR-URL: #17722
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2018
Merged
MylesBorins pushed a commit that referenced this pull request Feb 11, 2018
@BridgeAR @MylesBorins
doc: use dashes instead of asterisks
7487cb7 
PR-URL: #17722
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 12, 2018
@BridgeAR @MylesBorins
doc: use dashes instead of asterisks
6c2bfe8 
PR-URL: #17722
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 13, 2018
@BridgeAR @MylesBorins
doc: use dashes instead of asterisks
0ce48f9 
PR-URL: #17722
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
@BridgeAR BridgeAR deleted the doc-updates branch April 1, 2019 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardlau richardlau richardlau left review comments

@danbev danbev danbev approved these changes

@jasnell jasnell jasnell approved these changes

@lpinca lpinca lpinca approved these changes

@maclover7 maclover7 maclover7 approved these changes

Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@BridgeAR @richardlau @gibfahn @danbev @jasnell @lpinca @maclover7 @MylesBorins @nodejs-github-bot