Skip to content

doc: escape brackets not used as markdown reference links - #29809

Closed
nschonni wants to merge 1 commit into
nodejs:masterfrom
nschonni:escape-brackets-in-docs
Closed

doc: escape brackets not used as markdown reference links#29809
nschonni wants to merge 1 commit into
nodejs:masterfrom
nschonni:escape-brackets-in-docs

Conversation

@nschonni

@nschonninschonni commented Oct 2, 2019•
edited
Loading

Copy link
Copy Markdown
Member

Split from #29558

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-botnodejs-github-bot added the doc Issues and PRs related to the documentations. label Oct 2, 2019
@Trott

Trott commented Oct 4, 2019

Copy link
Copy Markdown
Member

@nodejs/documentation This could use some reviews.

BridgeAR
BridgeAR reviewed Oct 9, 2019
View reviewed changes

@BridgeARBridgeAR left a comment

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.

I am +-0 on this. I guess most collaborators think the same here?

@nschonni
nschonniforce-pushed the escape-brackets-in-docs branch 2 times, most recently from 2621fa3 to 080d8b1CompareOctober 9, 2019 22:49
nschonni
nschonni commented Oct 9, 2019
View reviewed changes
Comment threaddoc/api/crypto.md Outdated

@nschonninschonniOct 9, 2019•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

This escaping of the few instances [encoding] in the headings are the only functional difference that this rule caught. Before it was adding a hyperlink in the title because that bracket work is used as a reference link in this document

@nschonni

nschonni commented Oct 9, 2019

Copy link
Copy Markdown
MemberAuthor

Resolved the few merge issues and pointed out the only instance where this fixes a rendering issue

@nschonni
nschonniforce-pushed the escape-brackets-in-docs branch from 080d8b1 to 7eb0e21CompareOctober 9, 2019 23:26
@nschonni
doc: escape brackets not used as markdown reference links
d224e9b
These can turn into links if reference links are added to the document
@nschonni
nschonniforce-pushed the escape-brackets-in-docs branch from 7eb0e21 to d224e9bCompareOctober 10, 2019 05:18
@Trott

Trott commented Oct 11, 2019

Copy link
Copy Markdown
Member

Presumably there's a lint rule that can be added/enabled for this?

@Trott

Trott commented Oct 11, 2019

Copy link
Copy Markdown
Member

Lite CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3986/

@Trott

Trott commented Oct 11, 2019•
edited
Loading

Copy link
Copy Markdown
Member

There are probably many currently-open PRs that will add more unescaped brackets when they land. I'm OK with that, though, if there's a lint rule on the horizon. We can fix up those instances when the lint rule lands.

@TrottTrott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 11, 2019
@nschonni

nschonni commented Oct 11, 2019

Copy link
Copy Markdown
MemberAuthor

It would be covered by some of the rules in nodejs/remark-preset-lint-node#21 that are currently disabled. I'll go check to see if some of them can be re-enabled with the PRs that landed

@Trott

Trott commented Oct 11, 2019

Copy link
Copy Markdown
Member

Landed in 81bc7b3

@TrottTrott closed this Oct 11, 2019
Trott pushed a commit that referenced this pull request Oct 11, 2019
@nschonni@Trott
doc: escape brackets not used as markdown reference links
81bc7b3
These can turn into links if reference links are added to the document
PR-URL: #29809
Reviewed-By: Rich Trott <rtrott@gmail.com>
@nschonni

nschonni commented Oct 11, 2019•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Oops, missed 2 in fs.md after the rebase, but i'll enable the rule anyway and update that PR nevemind, seems OK

@nschonninschonni mentioned this pull request Oct 11, 2019
Merged
targos pushed a commit that referenced this pull request Oct 14, 2019
@nschonni@targos
doc: escape brackets not used as markdown reference links
b916ea3
These can turn into links if reference links are added to the document
PR-URL: #29809
Reviewed-By: Rich Trott <rtrott@gmail.com>
@nschonninschonni mentioned this pull request Oct 14, 2019
Merged
3 tasks
@targostargos mentioned this pull request Oct 15, 2019
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BridgeARBridgeARBridgeAR left review comments
@TrottTrottTrott approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.docIssues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nschonni@Trott@BridgeAR@nodejs-github-bot