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

Lint markdown in PRs #62

Merged
merged 6 commits into from
Dec 4, 2023
Merged

Conversation

gonX
Copy link
Member

@gonX gonX commented Nov 28, 2023

Lints markdown with markdownlint-cli with the GitHub action nosborn/github-action-markdown-cli.

Uses my local fork until a PR is merged that we need: nosborn/github-action-markdown-cli#64

Notes

  • Fragments to other pages does not seem to be verified (e.g. {% link /index.md %}#myid %}) less of a concern with workflows: Add HTML Verification #76
  • Code blocks after <ol>'s are not linted for indentation. We can't reasonably lint for this even with a custom rule so I've tried to make sure code blocks are indented correctly with this PR.

@gonX gonX added enhancement New feature or request ci Affects CI (Actions, bots, repo events, etc.) linting Affects linting for style and/or code meta Dotfiles, Scripts, Github files, Repository management labels Nov 28, 2023
@gonX gonX marked this pull request as ready for review November 28, 2023 16:27
@gonX gonX requested a review from X9VoiD November 28, 2023 16:27
@gonX

This comment was marked as outdated.

@gonX gonX changed the title Workflows lint markdown Lint markdown in PRs Nov 30, 2023
This was referenced Nov 30, 2023
@gonX gonX force-pushed the workflows-lint-markdown branch 3 times, most recently from 34fea04 to 9698d1f Compare December 2, 2023 09:40
@gonX
Copy link
Member Author

gonX commented Dec 2, 2023

Rebased to work with the new Jekyll 4.x deployment

Also adds a .markdownlint.yaml file for use with markdownlint-cli

We use a fork for the markdown-cli action

The original action uses an old version of markdownlint-cli that
triggers some false-positives with MD051 that doesn't support
predefined ID's for sections
@gonX gonX marked this pull request as draft December 3, 2023 07:26
This is a hack to get some specific styling on the website

Currently the hack lexer supports '#' comments, anything else is parsed as text
@gonX gonX marked this pull request as ready for review December 3, 2023 07:55
Copy link
Member

@X9VoiD X9VoiD left a comment

Choose a reason for hiding this comment

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

LGTM, just some style consistency issues.

site/_wiki/FAQ/Linux.md Outdated Show resolved Hide resolved
site/_wiki/FAQ/Linux.md Outdated Show resolved Hide resolved
@X9VoiD
Copy link
Member

X9VoiD commented Dec 3, 2023

Worth considering if we want to add markdown linting via docker image to avoid pulling node/npm:

docker run -v $PWD:/workdir ghcr.io/igorshubovych/markdownlint-cli:latest "*.md"

example taken from https://github.com/igorshubovych/markdownlint-cli

@gonX
Copy link
Member Author

gonX commented Dec 3, 2023

Worth considering if we want to add markdown linting via docker image to avoid pulling node/npm:

docker run -v $PWD:/workdir ghcr.io/igorshubovych/markdownlint-cli:latest "*.md"

example taken from https://github.com/igorshubovych/markdownlint-cli

Nice, will test which is faster before merging.

gonX and others added 2 commits December 3, 2023 16:42
Co-authored-by: Oscar Ivan Silvestre <[email protected]>
Co-authored-by: Oscar Ivan Silvestre <[email protected]>
@X9VoiD
Copy link
Member

X9VoiD commented Dec 4, 2023

I think its current state can be merged now as-is, but I'm leaving that decision to you @gonX since I'm not sure if you still have some other plans for this PR. Fwiw, the docker thing can just be done on a later PR as it doesn't change anything functionally.

@gonX
Copy link
Member Author

gonX commented Dec 4, 2023

Ok, makes sense

@gonX gonX merged commit d5d4b35 into OpenTabletDriver:main Dec 4, 2023
7 checks passed
@gonX gonX deleted the workflows-lint-markdown branch December 4, 2023 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Affects CI (Actions, bots, repo events, etc.) enhancement New feature or request linting Affects linting for style and/or code meta Dotfiles, Scripts, Github files, Repository management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants