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: sort bottom-of-file markdown links #24679

Closed
wants to merge 1 commit into from

Conversation

@sam-github
Copy link
Member

commented Nov 27, 2018

Reapply #12726

It would be nice to have the sort check applied as part of doc testing,
but this change doesn't implement that.

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
@nodejs-github-bot

This comment has been minimized.

Copy link

commented Nov 27, 2018

@sam-github sadly an error occured when I tried to trigger a build :(

sam-github added a commit to sam-github/node that referenced this pull request Nov 27, 2018
sam-github added a commit to sam-github/node that referenced this pull request Nov 27, 2018
sam-github added a commit to sam-github/node that referenced this pull request Nov 27, 2018
@sam-github

This comment has been minimized.

Copy link
Member Author

commented Nov 27, 2018

This won't land on 11, 10, 8, so I prepared backports immediately, see:

@vsemozhetbyt

This comment has been minimized.

Copy link
Contributor

commented Nov 27, 2018

We still have one inconsistency. In ASCII the order is: A-Z (upper case), ` (backtick), a-z (lower case). And we conform inside lines, even when it does not seem completely intuitively right (not as in dictionaries):

[`DeflateRaw`]: #zlib_class_zlib_deflateraw
[`Deflate`]: #zlib_class_zlib_deflate

However, as for the first symbol, some documents have upper case letters before backticks (i.e.
consistently):

[Using `options.selectPadding()`]: #http2_using_options_selectpadding
[`'checkContinue'`]: #http2_event_checkcontinue

some documents have upper case letters after backtick (i.e. not consistently):

[`strict mode`]: #assert_strict_mode
[Abstract Equality Comparison]: https://tc39.github.io/ecma262/#sec-abstract-equality-comparison
@sam-github

This comment has been minimized.

Copy link
Member Author

commented Nov 27, 2018

@vsemozhetbyt I didn't try to fix all inconsistencies, my goal was to make it locally consistent. There was a fair number of unsorted-by-any-algorithm positions.

Maybe that's a mistake. I would personally prefer to have strict ordering according to the default "C"/Posix locale -- which is ASCII ordering. Entirely ASCII makes it much faster to pass the entire block through programmer's editor's sort command, but IIRC from my first crack at this, existing usage had the backtick docs before all others, so I kept that in order to minimize churn.

@sam-github

This comment has been minimized.

Copy link
Member Author

commented Nov 27, 2018

Btw, I'm willing to go through and resort all the open sorting PRs, time consuming though it is, but only if I know they will be rubber stamped.

@vsemozhetbyt

This comment has been minimized.

Copy link
Contributor

commented Nov 27, 2018

If you mean the backport PRs, they still have no reviews, but I plan to review them. And I usually compare automatically-sorted-in-editor lists with your variants when the diff is big)

@vsemozhetbyt

This comment has been minimized.

Copy link
Contributor

commented Nov 27, 2018

Should I wait for resorting in the backporting PRs?

@sam-github

This comment has been minimized.

Copy link
Member Author

commented Nov 27, 2018

Should you wait? I don't know. If you are OK with this one landing, then yes, go ahead and review the others so they can all land at about the same time.

None of them are backports in the sense that I didn't use git to make them, there are too many differences between branches. I went through and sorted doc/api/*.md with my editor for all the branches to make sure they all end up in the same state, on all branches.

I think the current set of PRs makes all the files locally consistent, which isn't just cosmetic. Its sort of a flag day... any commits landing on master after this that touch the markdown refs will have trouble cherry-picking back to the branches, unless those branches are already sorted.

It would be really nice to have a markdown test tool that enforced ordering.

@vsemozhetbyt

This comment has been minimized.

Copy link
Contributor

commented Nov 27, 2018

I am OK with this one landing as is. I will try to review the others tomorrow.

@vsemozhetbyt

This comment has been minimized.

Copy link
Contributor

commented Nov 28, 2018

It seems this need rebasing to run CI.

sam-github added a commit to sam-github/node that referenced this pull request Nov 28, 2018
sam-github added a commit to sam-github/node that referenced this pull request Nov 28, 2018
sam-github added a commit to sam-github/node that referenced this pull request Nov 28, 2018

@sam-github sam-github force-pushed the sam-github:resort-markdown-refs branch from cfab420 to e208a16 Nov 28, 2018

@sam-github

This comment has been minimized.

Copy link
Member Author

commented Nov 28, 2018

PTAL

@vsemozhetbyt

This comment has been minimized.

@vsemozhetbyt

This comment has been minimized.

Copy link
Contributor

commented Nov 28, 2018

Node.js Collaborators, please, add 👍 here if you approve fast-tracking.

@sam-github

This comment has been minimized.

Copy link
Member Author

commented Nov 28, 2018

I'd like to fast-track to avoid conflicts.

doc/api/fs.md Outdated Show resolved Hide resolved
doc: sort bottom-of-file markdown links
Reapply #12726

It would be nice to have the sort check applied as part of doc testing,
but this change doesn't implement that.

@sam-github sam-github force-pushed the sam-github:resort-markdown-refs branch from e208a16 to f34fede Nov 28, 2018

@Trott Trott closed this Nov 28, 2018

sam-github added a commit to sam-github/node that referenced this pull request Nov 29, 2018
sam-github added a commit to sam-github/node that referenced this pull request Nov 30, 2018
sam-github added a commit to sam-github/node that referenced this pull request Nov 30, 2018
rvagg added a commit that referenced this pull request Dec 1, 2018
doc: sort bottom-of-file markdown links
Backport of #24679

PR-URL: #24682
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
sam-github added a commit to sam-github/node that referenced this pull request Dec 3, 2018
doc: sort bottom-of-file markdown links
Backport-PR-URL: nodejs#24680
PR-URL: nodejs#24679
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
sam-github added a commit to sam-github/node that referenced this pull request Dec 3, 2018
BridgeAR added a commit that referenced this pull request Dec 5, 2018
doc: sort bottom-of-file markdown links
Backport-PR-URL: #24680
PR-URL: #24679
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@BridgeAR BridgeAR referenced this pull request Dec 5, 2018
4 of 4 tasks complete
sam-github added a commit to sam-github/node that referenced this pull request Dec 17, 2018
MylesBorins added a commit that referenced this pull request Dec 21, 2018
doc: sort bottom-of-file markdown links
Reapply #12726

It would be nice to have the sort check applied as part of doc testing,
but this change doesn't implement that.

Backport-PR-URL: #24681
PR-URL: #24679
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins added a commit that referenced this pull request Dec 26, 2018
doc: sort bottom-of-file markdown links
Reapply #12726

It would be nice to have the sort check applied as part of doc testing,
but this change doesn't implement that.

Backport-PR-URL: #24681
PR-URL: #24679
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@codebytere codebytere referenced this pull request Jan 4, 2019
refack added a commit to refack/node that referenced this pull request Jan 14, 2019
doc: sort bottom-of-file markdown links
Reapply nodejs#12726

It would be nice to have the sort check applied as part of doc testing,
but this change doesn't implement that.

PR-URL: nodejs#24679
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

@sam-github sam-github deleted the sam-github:resort-markdown-refs branch Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.