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

Add make format to lint Markdown #267

Closed
MaxGhenis opened this issue Jun 12, 2022 · 3 comments · Fixed by #277
Closed

Add make format to lint Markdown #267

MaxGhenis opened this issue Jun 12, 2022 · 3 comments · Fixed by #277

Comments

@MaxGhenis
Copy link
Contributor

Do the docs use a common markdown formatter? In #266 VSCode automatically formatted the file. I'd be happy to use a different formatter, or I can start over there and avoid the autoformatter, but a make format would be convenient.
@MattiSG
Copy link
Member

MattiSG commented Jun 16, 2022

Thanks @MaxGhenis! No, we don't have a linter set up on Markdown, and I agree it makes reviews more cumbersome and adds noise to every PR.

I would very much welcome a PR setting up an editorconfig, and even better adding markdownlint or equivalent in CI!

@MattiSG MattiSG changed the title Add make format Add make format to lint Markdown Jun 16, 2022
RobinCsl added a commit to RobinCsl/openfisca-doc that referenced this issue Oct 28, 2022
@RobinCsl
feat: add .editorconfig with Markdown specific rules
7b81e79 
Source: http://jonasbn.github.io/til/editorconfig/markdown.html

Contributes to openfisca#267
RobinCsl added a commit to RobinCsl/openfisca-doc that referenced this issue Oct 28, 2022
@RobinCsl
Introduce .editorconfig with Markdown specific rules
224c322 
Source: http://jonasbn.github.io/til/editorconfig/markdown.html

Contributes to openfisca#267
RobinCsl added a commit to RobinCsl/openfisca-doc that referenced this issue Oct 28, 2022
@RobinCsl
Introduce lint job in CI
495636e 
This commit adds a lint job to the CI pipeline. It uses an up
to date Markdown linter https://github.com/DavidAnson/markdownlint-cli2.

I looked for an alternative from the Python ecosystem (to stay
more consistent with this project), but the only result was an
outdated and unmaintained project.

It would be possible to add a `make format` command as was
suggested in the original GitHub issue, but this would require
Node.js to be available in the dev environment, so I thought
setting up CI only would be good enough for now.

Note: it should be as easy as adding the following to the
Makefile:
```
format:
	npx markdownlint-cli2-fix "**/*.md"
```

Closes openfisca#267
RobinCsl added a commit to RobinCsl/openfisca-doc that referenced this issue Oct 28, 2022
@RobinCsl
Introduce .editorconfig with Markdown specific rules
3ee79c4 
Source: http://jonasbn.github.io/til/editorconfig/markdown.html

Contributes to openfisca#267
RobinCsl added a commit to RobinCsl/openfisca-doc that referenced this issue Oct 28, 2022
@RobinCsl
Introduce lint job in CI
9781c61 
This commit adds a lint job to the CI pipeline. It uses an up
to date Markdown linter https://github.com/DavidAnson/markdownlint-cli2.

I looked for an alternative from the Python ecosystem (to stay
more consistent with this project), but the only result was an
outdated and unmaintained project.

It would be possible to add a `make format` command as was
suggested in the original GitHub issue, but this would require
Node.js to be available in the dev environment, so I thought
setting up CI only would be good enough for now.

Note: it should be as easy as adding the following to the
Makefile:
```
format:
	npx markdownlint-cli2-fix "**/*.md"
```

Closes openfisca#267
@RobinCsl RobinCsl mentioned this issue Oct 28, 2022
Merged
@RobinCsl
Copy link
Contributor

RobinCsl commented Oct 28, 2022 •
edited
Loading

Hi @MattiSG, I gave this issue a shot.

It's in #277. The job fails because there would be lots of Markdown errors to deal with.

One way to solve that could be to introduce a config file for the linter so that you disable the rules you can't fix for now.

Hope that helps!

@MattiSG
Copy link
Member

MattiSG commented Nov 3, 2022

Thank you so much @RobinCsl for your contribution, and sorry for the delay in handling it (I was off the last two weeks)! I just checked it out and set up Markdownlint to match the existing style, and fixed all (very relevant) errors.

We thus will get the following benefits, beyond the linting itself:

  • Unified header and lists styles and whitespace.
  • All fenced blocks have syntax highlighting.

@MattiSG MattiSG mentioned this issue Nov 3, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Introduce lint job in CI RobinCsl/openfisca-doc
3 participants
@MattiSG @MaxGhenis @RobinCsl