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

fix: no-shortcut-reference-link #2697

Merged
merged 3 commits into from
May 2, 2020

Conversation

nschonni
Copy link
Member

No description provided.

locale/en/blog/release/v0.10.45.md Outdated Show resolved Hide resolved
@XhmikosR
Copy link
Contributor

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

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
@@ -7,6 +7,10 @@ slug: node-v0-10-42
layout: blog-post.hbs
author: James M Snell
---
<!--lint disable prohibited-strings-->
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

@@ -139,7 +139,7 @@ node --prof-process isolate-0xnnnnnnnnnnnn-v8.log > processed.txt
3165 8.4% 8.6% _malloc_zone_malloc
```

我们看到,前 3 个条目占了程序占用的 CPU 时间的 72.1%。从这个输出中,我们立即看到至少 51.8% 的 CPU 时间被称为 PBKDF2 的函数占用。它与用户密码中的哈希生成相对应。然而,较低的两个条目的因素是如何进入我们的应用程序(或者我们为了例子而假装如此)不会立即明显得看出来。为了更好地理解这些函数之间的关系,接下来我们将查看[自下而上(重)配置文件]部分,该节提供有关每个函数的主要调用方的信息。检查此部分,我们会发现:
我们看到,前 3 个条目占了程序占用的 CPU 时间的 72.1%。从这个输出中,我们立即看到至少 51.8% 的 CPU 时间被称为 PBKDF2 的函数占用。它与用户密码中的哈希生成相对应。然而,较低的两个条目的因素是如何进入我们的应用程序(或者我们为了例子而假装如此)不会立即明显得看出来。为了更好地理解这些函数之间的关系,接下来我们将查看\[自下而上(重)配置文件\]部分,该节提供有关每个函数的主要调用方的信息。检查此部分,我们会发现:
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants