Skip to content

fix: no-shortcut-reference-link #2697

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
Merged
merged 3 commits into from
May 2, 2020
Merged

fix: no-shortcut-reference-link #2697

merged 3 commits into from
May 2, 2020

Conversation

nschonni
Copy link
Member

No description provided.

XhmikosR
XhmikosR suggested changes Oct 18, 2019
View reviewed changes
@XhmikosR
Copy link
Contributor

@nschonni can you fix tests and take care of my comment above?

@XhmikosR XhmikosR mentioned this pull request Nov 13, 2019
Merged
MylesBorins
MylesBorins reviewed Dec 20, 2019
View reviewed changes
Copy link
Contributor

@MylesBorins MylesBorins left a comment

Choose a reason for hiding this comment

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

The changelogs are generated from the upstream changelog in the repo itself. I'm not 100% that we should be changing them for a linting rule

@Trott
Copy link
Member

Trott commented Dec 20, 2019

The changelogs are generated from the upstream changelog in the repo itself. I'm not 100% that we should be changing them for a linting rule

Since we're importing them from elsewhere, it may be best to simply skip linting on them entirely and enforce any lint rules we want upstream (or just don't worry about the linting for the changelogs).

@nschonni nschonni force-pushed the fix--no-shortcut-reference-link branch 3 times, most recently from bb32b11 to 37e923f Compare May 1, 2020 00:54
@nschonni nschonni requested a review from XhmikosR May 1, 2020 00:55
@nschonni
Copy link
Member Author

nschonni commented May 1, 2020

Although a bunch of the changelog related fixes have been applied upstream, i've just disabled the same rules plus this new one at the file level for the changlog blogs

@nschonni nschonni force-pushed the fix--no-shortcut-reference-link branch from 37e923f to 3ebf376 Compare May 1, 2020 01:13
@nschonni nschonni mentioned this pull request May 1, 2020
Merged
XhmikosR
XhmikosR reviewed May 1, 2020
View reviewed changes
locale/en/blog/release/v0.10.42.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you confirm such comments are not in the built HTML? Not a blocker, but we should be aware about it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it looks like it ends up the built HTML

XhmikosR
XhmikosR reviewed May 1, 2020
View reviewed changes
locale/zh-cn/docs/guides/simple-profiling.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

This is Chinese to me so I can't judge the change 😛

@XhmikosR
Copy link
Contributor

XhmikosR commented May 1, 2020

LGTM now, it needs a rebase and I left a couple of comments. I can approve after those are addressed 🙂

@nschonni nschonni force-pushed the fix--no-shortcut-reference-link branch from 3ebf376 to 7c4f3b1 Compare May 1, 2020 16:56
@nschonni
Copy link
Member Author

nschonni commented May 1, 2020

Rebased. Confirmed that the remark-lint-no-literal-urls will need additional work in a separate PR to re-enable

@nschonni nschonni requested a review from XhmikosR May 1, 2020 17:10
@nschonni nschonni merged commit 18b0418 into nodejs:master May 2, 2020
@nschonni nschonni deleted the fix--no-shortcut-reference-link branch May 2, 2020 00:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@MylesBorins MylesBorins MylesBorins left review comments

@XhmikosR XhmikosR XhmikosR approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nschonni @XhmikosR @Trott @MylesBorins