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

*: check connectivity of external links via GitHub action #353

Merged
merged 30 commits into from
Sep 7, 2020

Conversation

csuzhangxc
Copy link
Member

@csuzhangxc csuzhangxc commented Sep 7, 2020

What is changed, added or deleted? (Required)

use https://github.com/marketplace/actions/link-checker to check links in zh and en.

Note this external link check isn't required to pass, in order to prevent false alarms from blocking PR merge.

The current regex exclusions is:

http://172.*|https://github.com/pingcap/dm/pull/.*|.*.md

Meaning the check will skip checking the URLs matching the above patterns.

Which DM version(s) do your changes apply to? (Required)

  • master (the latest development version, including v3.0 changes)
  • v2.0 (TiDB DM 2.0 versions)
  • v1.0 (TiDB DM 1.0 versions)

What is the related PR or file link(s)?

  • This PR is translated from:
  • Other reference link(s):

Do your changes match any of the following descriptions?

  • Delete files
  • Change aliases
  • Have version specific changes
  • Might cause conflicts after cherry-picked

@csuzhangxc csuzhangxc changed the title *: check link via GitHub action [WIP] *: check link via GitHub action Sep 7, 2020
- name: Link check for zh
uses: peter-evans/link-checker@v1
with:
args: -c 4 -d /github/workspace -r -v -x http://172.* zh*
Copy link
Member Author

Choose a reason for hiding this comment

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

github 太容易被触发 Too Many Requests (HTTP error 429) 了。但这里并发已经调整到 4 了,再降低怕影响速度

Copy link
Contributor

Choose a reason for hiding this comment

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

是因为查了很多 https://github.com/pingcap/dm/pull/xxx 导致 429 的吗?这类链接 404 的概率应该很小,可以考虑不查这种链接

@csuzhangxc
Copy link
Member Author

@lance6716 @yikeke PTAL

@csuzhangxc csuzhangxc changed the title [WIP] *: check link via GitHub action *: check link via GitHub action Sep 7, 2020
# but if `-c` is too large then `Too Many Requests (HTTP error 429)` may be reported from `https://github.com/*`.
# - we hardcode `--document-root` to `/github/workspace` in the container now.
# - we use `http://172.*` as sample addresses in some docs, so we need to exclude them.
- name: Link check for zh
Copy link
Contributor

Choose a reason for hiding this comment

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

新 workflow 的功能是内链(相对链接)和外链(绝对链接)都会查吗 @csuzhangxc

Copy link
Member Author

Choose a reason for hiding this comment

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

是的。但不能正确检测出锚文本

Copy link
Contributor

@yikeke yikeke Sep 7, 2020

Choose a reason for hiding this comment

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

内链 和 内链锚文本 的检查都已经有了,在 ci.yaml 的 Verify links 和 Verify link anchors 两个 step 中实现了。学程哥新加的这个可以配置成只检查 外链 吗

Copy link
Contributor

@yikeke yikeke Sep 7, 2020

Choose a reason for hiding this comment

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

如果方便的话,最好把检查外链的这个 action 也写到 ci.yaml 那个文件里,写成和 Verify links、Verify link anchors 并列的一个新的 step,就不要另起一个 workflow 了,怎么样? @csuzhangxc

Copy link
Member Author

Choose a reason for hiding this comment

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

想的是单开一个 workflow,不是 Required。这样检查异常时,也还是能 merge?

Copy link
Contributor

Choose a reason for hiding this comment

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

嗯,这样的话也可以,就不必因为 pr 未涉及的 404 外链而 block merge 了

@yikeke
Copy link
Contributor

yikeke commented Sep 7, 2020

报错的信息可以过滤下不?现在成功的也会报出来,把 error 的给淹没了

@yikeke yikeke added needs-cherry-pick-release-1.0 Should cherry pick this PR to release-1.0 branch. needs-cherry-pick-release-2.0 Should cherry pick this PR to release-2.0 branch. translation/no-need The changes in this PR don't need to be translated. labels Sep 7, 2020
yikeke added 2 commits September 7, 2020 15:28
This reverts commit 81661a9.
Copy link
Contributor

@yikeke yikeke left a comment

Choose a reason for hiding this comment

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

LGTM. Good job!

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 7, 2020
@yikeke yikeke merged commit 6ae07ea into pingcap:master Sep 7, 2020
ti-srebot pushed a commit to ti-srebot/docs-dm that referenced this pull request Sep 7, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-1.0 in PR #356

ti-srebot pushed a commit to ti-srebot/docs-dm that referenced this pull request Sep 7, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-2.0 in PR #357

@csuzhangxc csuzhangxc deleted the link-check branch September 7, 2020 07:49
yikeke pushed a commit that referenced this pull request Sep 7, 2020
* cherry pick #353 to release-2.0

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

* Update ci.yaml

Co-authored-by: Xuecheng Zhang <csuzhangxc@gmail.com>
Co-authored-by: yikeke <yikeke@pingcap.com>
yikeke added a commit that referenced this pull request Sep 7, 2020
* cherry pick #353 to release-1.0

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

* Update ci.yaml

* Delete maintain-dm-using-tiup.md

Co-authored-by: Xuecheng Zhang <csuzhangxc@gmail.com>
Co-authored-by: yikeke <yikeke@pingcap.com>
Co-authored-by: Keke Yi <40977455+yikeke@users.noreply.github.com>
@yikeke yikeke changed the title *: check link via GitHub action *: check connectivity of external links via GitHub action Sep 9, 2020
args: -c 32 -d /github/workspace -r -x "http://172.*|https://github.com/pingcap/dm/pull/.*|.*.md" zh*

- name: Check external links in en files
id: lc
Copy link
Contributor

Choose a reason for hiding this comment

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

请教下,上面的「Check external links in zh files」那一步没有 id,如果只有 zh 有 error、en 没有 error,下面的 ${{ steps.lc.outputs.exit_code }} 会是 1 吗? @csuzhangxc

Copy link
Member Author

Choose a reason for hiding this comment

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

啊。我的错误,都需要加 id、都需要判断

Copy link
Contributor

Choose a reason for hiding this comment

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

嗯嗯,学程哥帮忙提个 pr 改下?

Copy link
Member Author

Choose a reason for hiding this comment

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

#370 PTAL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-cherry-pick-release-1.0 Should cherry pick this PR to release-1.0 branch. needs-cherry-pick-release-2.0 Should cherry pick this PR to release-2.0 branch. status/LGT1 Indicates that a PR has LGTM 1. translation/no-need The changes in this PR don't need to be translated.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants