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

Introduce lint job in CI #277

Merged
merged 19 commits into from
Nov 3, 2022
Merged

Introduce lint job in CI #277

merged 19 commits into from
Nov 3, 2022

Conversation

RobinCsl
Copy link
Contributor

Introduce lint job in CI

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 #267

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
Copy link
Member

@MattiSG MattiSG left a comment

Choose a reason for hiding this comment

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

Thank you very much @RobinCsl! I handled all detected Markdown errors and added a bit of documentation in the README on how to use the commands make lint and make format.

Thank you for having implemented two separate jobs in CI, this really adds to the readability of the feedback 👍 I will proceed with making the lint job mandatory as well 🙂

@MattiSG
Copy link
Member

MattiSG commented Nov 3, 2022

it should be as easy as adding the following to the Makefile

Thanks for the hint! I tried to implement it this way but had to switch to markdownlint-cli since markdownlint-cli2-fix is not available over NPM (see de0b586).

@MattiSG MattiSG merged commit bc5c841 into openfisca:master Nov 3, 2022
@RobinCsl RobinCsl deleted the rcsl/fix-267 branch November 3, 2022 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add make format to lint Markdown
2 participants