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.

Sign up for GitHub

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

Jump to bottom

Lint Markdown with pre-commit hook #700

Open
ns-rse opened this issue Jul 19, 2023 · 2 comments · May be fixed by #701
Open

Lint Markdown with pre-commit hook #700

ns-rse opened this issue Jul 19, 2023 · 2 comments · May be fixed by #701
Labels
question Further information is requested

Comments

@ns-rse
Copy link
Contributor

ns-rse commented Jul 19, 2023 •
edited
Loading

Continuing my drive to get everyone hooked on pre-commit I was wondering how people would feel about having the markdownlint-cli2 pre-commit hook in place and in the CI pipeline (via pre-commit.ci) to lint our Markdown when we post new articles.

I wouldn't propose going back through the older posts as that would take some considerable time!
@ns-rse ns-rse added the question Further information is requested label Jul 19, 2023
@Robadob
Copy link
Member

Robadob commented Jul 19, 2023

Good idea, assuming the lint rules are sensible.

I wouldn't propose going back through the older posts as that would take some considerable time!

Is there not a tool that would auto apply lint rules to cleanup old posts? I imagine much of it could be done relatively easy with some find/replace once identified, I did similar when we first setup lint in FLAMEGPU2.

@ns-rse
Copy link
Contributor Author

ns-rse commented Jul 19, 2023 •
edited
Loading

I've found some of the rules to be a bit aggressive but its possible to configure which you use.

The markdownlint-cli2 that I've used (and the other related packages) only appear to tell you what the problems are, rather than correcting them. If they were corrected we'd also have to ignore the git blame for whoever makes those changes (but I read something somewhere that tells you how to do that).

ns-rse added a commit that referenced this issue Jul 22, 2023
@ns-rse
Introducing pre-commit and markdownlint-cli2
1cafc72 
Closes #700

Adds [pre-commit](https://pre-commit.com) to the repository (and configures but doesn't yet enable
[pre-commit.ci](https://pre-commit.ci)). Some basic `pre-commit` hooks are included but the meat of the linting is done
via [markdownlint-cli2](https://github.com/DavidAnson/markdownlint-cli2) which is configured via
`.markdownlint-cli2.yaml`.

Retrospective linting of the `.md` and `.sh` files included in this PR has been undertaken (mostly line-length issues,
but a typo has been corrected in `_posts/2022-12-22-git-blame.md` too).

`README.md` has a new section on setting up and using `pre-commit` for this work and includes details of why
retrospective linting has not been applied more widely (basically I didn't want to break any pages) and how to ignore
revisions should anyone wish to undertake this task.

`markdownlint-cli2` does have an option to `fix: true` which will where possible fix things automatically, but I had a
go at doing this and it wasn't applying things very well (e.g. line lengths weren't being sorted). I have written a
short script ([`wrap`](https://gitlab.com/nshephard/dotfiles/-/blob/master/bin/wrap)) that uses `fmt` to sensibly wrap
lines for all files of a particular type at a specified length (and in parallel too) if anyone wanted to have a go at
sorting things out.

One thing to discuss would be whether we enable [pre-commit.ci](https://pre-commit.ci/). I didn't want to foist this
upon people without consensus being reached.
@ns-rse ns-rse linked a pull request Jul 22, 2023 that will close this issue
Open
@ns-rse ns-rse mentioned this issue Nov 2, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
question Further information is requested
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Introducing pre-commit and markdownlint-cli2 RSE-Sheffield/RSE-Sheffield.github.io
2 participants
@Robadob @ns-rse