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

Write test for bad pattern: .md/# #23329

Closed
HonkingGoose opened this issue Jul 12, 2023 · 13 comments · Fixed by #23609
Closed

Write test for bad pattern: .md/# #23329

HonkingGoose opened this issue Jul 12, 2023 · 13 comments · Fixed by #23609
Labels
good first issue Suitable for new contributors help wanted Help is needed or welcomed on this issue priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others type:feature Feature (new functionality)

Comments

@HonkingGoose
Copy link
Collaborator

Describe the proposed change(s).

Write or adjust a GitHub Action test, that fails if it finds the .md/# pattern in any of our .md files.

Optional: the Action writes an annotation to the bad file. Like this:

The .md/# pattern is bad, remove the / beween the filename and the anchor. Correct: .md#section-name.

Describe why we need/want these change(s).

I'm writing bad links each time, where I use the .md/# pattern to link to another file. I'd like a test that stops me doing this stupid thing. 😄

This is because the VSCode preview accepts the bad pattern, but our published docs get messed up. 🙈

@HonkingGoose HonkingGoose added type:feature Feature (new functionality) priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others status:ready labels Jul 12, 2023
@viceice
Copy link
Member

viceice commented Jul 12, 2023

I think we can use a simple jest test to find files with those wrong links and fails if one is found.

@HonkingGoose
Copy link
Collaborator Author

That sounds good to me! 😄

@HonkingGoose HonkingGoose changed the title Write GitHub Action test for bad pattern: .md/# Write test for bad pattern: .md/# Jul 12, 2023
@HonkingGoose HonkingGoose added good first issue Suitable for new contributors help wanted Help is needed or welcomed on this issue labels Jul 17, 2023
@lmilbaum
Copy link
Contributor

I'd suggest using pre-commit hook - markdown-link-check - https://github.com/tcort/markdown-link-check

@HonkingGoose
Copy link
Collaborator Author

HonkingGoose commented Jul 29, 2023

MkDocs v1.5.0 comes with better link validation. 1 I don't know if that would catch my .md/# style bad links?

One problem remains, the MkDocs build step runs at the end, not at the beginning. Here's what I mean:

  1. We update to MkDocs v1.5.0 and enable link validation, and assume it will catch the bad link pattern
  2. @HonkingGoose writes bad link pattern
  3. Renovate updates the renovatebot/renovate Git submodule on the docs repository
  4. Our CI runs the mkdocs build in the strict mode, and it errors out
  5. @HonkingGoose fixes bad link in the renovatebot/renovate repository
  6. Renovate updates submodule again, with the good link
  7. MkDocs build passes
  8. Updated site is published

We probably still want some kind of check to find the bad links when I'm writing them... 🙈

Footnotes

  1. MkDocs 1.5.0 release notes, expanded validation of links

@HonkingGoose
Copy link
Collaborator Author

Pinging @PeterNitsche so they see the new comments in this issue.

@lmilbaum
Copy link
Contributor

pre-commit can catch mistakes when one tries to commit (before the code changes pushed to github). @HonkingGoose Would that cover your ask?

@HonkingGoose
Copy link
Collaborator Author

I sometimes "cheat" by using the GitHub code editor online, instead waiting for a fresh Codespace to start. So using a pre-commit check only won't catch all my bad stuff. Whatever tests we're going to use/build, we should run automatically. 😄

@lmilbaum
Copy link
Contributor

The same pre-commit process can also invoked in CI. See this example: https://github.com/devopsloft/devopsloft.github.io/blob/main/.github/workflows/pre_commit.yml

@viceice
Copy link
Member

viceice commented Jul 29, 2023

pre-commit hooks where removed recently because of slowness, so they aren't an option again

@PeterNitsche
Copy link
Contributor

Funny, the issue remains untouched for 2 weeks with a proposed solution and once the proposal is implemented, other proposals are discussed. 😄

Regarding the discussion: I like the pragmatic solution of using Jest to validate the documentation. It fits into the code concept since there are already other Jest tests validating the documentation.
Adding another library to the stack means also increased complexity, maintenance effort and risk of vulnerabilities in the future. The Jest solution solves exactly the problem described by @HonkingGoose and nothing more.
If @HonkingGoose notices more issues with invalid markdown files, a third-party library can be added in another iteration.

@HonkingGoose
Copy link
Collaborator Author

I'm happy with fixing just my bad links in the first iteration. 🙂

I'm only mentioning the mkdocs update because it might affect how/if the maintainers want to fix this issue. 🙂 And when I wrote the issue, the mkdocs update wasn't out yet... 😇

@lmilbaum
Copy link
Contributor

Funny, the issue remains untouched for 2 weeks with a proposed solution and once the proposal is implemented, other proposals are discussed. 😄

I assume this comment is directed at me. I thank you for the feedback. It is completely valid.
I have good explanations why I haven't been engaged until now but maybe it's not that interesting.
In my honest opinion, in an open source project, one can be involved at any given moment and even after the fact. What to do with the proposals/feedback is up to the maintainers' decision (just for information, I am not one of them).

@PeterNitsche
Copy link
Contributor

@lmilbaum That comment was not meant as direct feedback at all. I am just highlighting the coincidental series of events which amuses me. 🙂

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Suitable for new contributors help wanted Help is needed or welcomed on this issue priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others type:feature Feature (new functionality)
Projects
None yet
4 participants