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

feat: add markdown link checking #1713

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

miguelsorianod
Copy link
Contributor

Description

This PR adds the ability for developers to check for broken link references in markdown files locally and it also integrates markdown link checking into the CI/CD process of this repository.

Details

Local development

This set of changes adds two Makefile targets

  • markdown-link-check-install: installs the markdown-link-check
    tool which is able to check whether links are alive
  • lint/markdown-link-check: Runs markdown-link-check checking
    all .md (markdown) files in the project. The bin directory is
    excluded

markdown-link-check is able to check:

  • URLs
  • Relative links to other files in the projects
  • mailto: links

Additionally, a markdown-link-check configuration file
.markdown-link-check/markdown-link-check-config.json relative
to the project's root has been added. This file can be used
when running the markdown-link-check tool with the -c flag
referencing it. The lint/markdown-link-check Makefile target
makes use of it automatically.

Some URL patterns have been configured to be ignored in the
markdown-link-check configuration file. This URL patterns
correspond to URLs that resolve to private IP addresses or that
are private GitHub/GitLab repositories.

CI/CD process

Two GitHub workflows have been added that perform Markdown links
checking:

  • .github/workflows/markdown-link-check-pr.yaml: Executed
    in each PR.
  • .github/workflos/markdown-link-check-weekly.yaml: Executed
    Weekly on Mondays at 07:00 UTC.

All the specified md files and directories in the workflows are
intentionally checked, independently on whether they have been
modified or not. The reason for this is that even if a given .md
file has not been modified, its links can become broken. For example,
if a PR moves .md files from directories and some other .md files
contain relative links referencing the old path of the moved files,
then the links become broken even though the file containing the
references not being modified.

Verification Steps

  • The CI task to check markdown links is executed and passes
  • Run markdown link checking locally by running make lint/markdown-lint-check. No errors should be reported.

Checklist (Definition of Done)

  • All acceptance criteria specified in JIRA have been completed
  • Unit and integration tests added that prove the fix is effective or the feature works (tested against emulated and non-emulated OCM environment)
  • Documentation added for the feature
  • CI and all relevant tests are passing
  • Code Review completed
  • Verified independently by reviewer
  • All PR comments are resolved either by addressing them or creating follow up tasks
  • Required metrics/dashboards/alerts have been added (or PR created).
  • Required Standard Operating Procedure (SOP) is added.
  • JIRA has been created for changes required on the client side

This set of changes adds two Makefile targets
* markdown-link-check-install: installs the markdown-link-check
  tool which is able to check whether links are alive
* lint/markdown-link-check: Runs markdown-link-check checking
  all .md (markdown) files in the project. The bin directory is
  excluded

markdown-link-check is able to check:
* URLs
* Relative links to other files in the projects
* `mailto:` links

Additionally, a markdown-link-check configuration file
`.markdown-link-check/markdown-link-check-config.json` relative
to the project's root has been added. This file can be used
when running the markdown-link-check tool with the `-c` flag
referencing it. The `lint/markdown-link-check` Makefile target
makes use of it automatically.

Some URL patterns have been configured to be ignored in the
markdown-link-check configuration file. This URL patterns
correspond to URLs that resolve to private IP addresses or that
are private GitHub/GitLab repositories.
@miguelsorianod miguelsorianod requested review from a team as code owners April 17, 2023 14:48
@github-actions github-actions bot added common documentation Improvements or additions to documentation github labels Apr 17, 2023
@@ -0,0 +1,38 @@
name: PR check Markdown links
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Separating this into its own workflow is intentional. The reason for it is that we want to take advantage of parallelism and restrict the set of events that are considered.
The same applies to the workflow for the periodic execution

use-quiet-mode: 'no'
use-verbose-mode: 'yes'
config-file: './.markdown-link-check/markdown-link-check-config.json'
check-modified-files-only: 'no'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is intentional:

All the specified md files and directories in the workflows are
intentionally checked, independently on whether they have been
modified or not. The reason for this is that even if a given .md
file has not been modified, its links can become broken. For example,
if a PR moves .md files from directories and some other .md files
contain relative links referencing the old path of the moved files,
then the links become broken even though the file containing the
references not being modified.

@@ -263,6 +280,10 @@ lint/templates: specinstall
$(SPECTRAL) lint templates/*.yml templates/*.yaml --ignore-unknown-format --ruleset .validate-templates.yaml
.PHONY: lint/templates

MARKDOWN_LINK_CHECK_CONFIG_DIR ?= $(PROJECT_PATH)/.markdown-link-check
MARKDOWN_LINK_CHECK_CONFIG ?= $(MARKDOWN_LINK_CHECK_CONFIG_DIR)/markdown-link-check-config.json
lint/markdown-link-check: markdown-link-check-install
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This hasn't been added to the lint task intentionally. As it might be time consuming I kept it separate from there.

@machi1990
Copy link
Contributor

.github/workflos/markdown-link-check-weekly.yaml: Executed
Weekly on Mondays at 07:00 UTC.

What's the motivation for running the check on a weekly basis on top of having the check run on every PR? I presume it is to detect any broken external link?

@miguelsorianod
Copy link
Contributor Author

miguelsorianod commented Apr 17, 2023

What's the motivation for running the check on a weekly basis on top of having the check run on every PR? I presume it is to detect any broken external link?

Correct. A link reference to a URL can become broken even if no PRs are opened.

Copy link
Contributor

@JameelB JameelB left a comment

Choose a reason for hiding this comment

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

Verified on MacOS. Works as expected, no dead links found.

@miguelsorianod miguelsorianod requested a review from a team April 17, 2023 15:21
Copy link
Contributor

@machi1990 machi1990 left a comment

Choose a reason for hiding this comment

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

left a few nitpicks in form of suggestions.
lgtm, thanks for this!

Two GitHub workflows have been added that perform Markdown links
checking:
* .github/workflows/markdown-link-check-pr.yaml: Executed
  in each PR.
* .github/workflos/markdown-link-check-weekly.yaml: Executed
  Weekly on Mondays at 07:00 UTC.

All the specified md files and directories in the workflows are
intentionally checked, independently on whether they have been
modified or not. The reason for this is that even if a given .md
file has not been modified, its links can become broken. For example,
if a PR moves .md files from directories and some other .md files
contain relative links referencing the old path of the moved files,
then the links become broken even though the file containing the
references not being modified.
@codecov
Copy link

codecov bot commented Apr 17, 2023

Codecov Report

Merging #1713 (a0c6b4c) into main (7e9dd15) will decrease coverage by 0.01%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1713      +/-   ##
==========================================
- Coverage   82.19%   82.19%   -0.01%     
==========================================
  Files         161      161              
  Lines       14999    14997       -2     
==========================================
- Hits        12329    12327       -2     
  Misses       2235     2235              
  Partials      435      435              
Flag Coverage Δ
unittests 82.19% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 3 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
common documentation Improvements or additions to documentation github
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants