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

CI: Codacy static analysis began reporting errors for Markdown lines greater than 80 char-length. #1185

Closed
jayaddison opened this issue Jul 25, 2024 · 6 comments

Comments

@jayaddison
Copy link
Collaborator

In pull request #1184 I've suggested some modifications to the documentation in our docs directory, which is written in Markdown format.

Our continuous integration runs some static analysis via a service called Codacy, and it currently reports a failure to the pull request continuous integration when Markdown lines are edited and exceed 80 characters in length:

image

image

Our documentation style has many existing lines that are already over 80-chars, and I don't think that we should perform a rewrite of the docs only to fit within them, nor should that single PR adhere to the 80-char limit, because that'd seem inconsistent.

So: I'm trying to figure out whether it's possible to configure Codacy to disable that line-length check -- perhaps selectively for Markdown files and/or the docs directory in this repository. Based on noticing an error code MD013 mentioned when viewing Codacy's details page for the failure, it seems likely to be a tool called markdownlint (there are at least two popular tools by that name, https://github.com/markdownlint/markdownlint and https://github.com/DavidAnson/markdownlint/ ).

After a couple of false starts (first, second, I've found another open source repository where this particular rule was disabled using a .codacy.yml file, so I think I'll try using the same approach next.

@jayaddison
Copy link
Collaborator Author

@hhursev @jknndy @strangetom I might need some help with this one; I've tried a few different ways but can't seem to disable the Codacy Static Analysis errors due to line-length on #1184.

The errors only occur for the text lines I've modified -- I could rewrite them to fit within 80-chars, but that'd seem inconsistent with the rest of the docs. I suppose rewriting all the docs to fit within 80-chars could be an alternative, though.

@jknndy
Copy link
Collaborator

jknndy commented Jul 26, 2024

One option would be excluding the docs/ directory from the Codacy run as outlined here, which would still require a .codacy.yml

I do wonder if it would be beneficial to rework all the docs to meet the 80 character limit anyways?

@strangetom
Copy link
Collaborator

strangetom commented Jul 26, 2024

I think I'm in favour of excluding the docs/ directory using .codacy.yml, or excluding all *.md files.

Running static analysis on markdown files doesn't seem like a useful thing to be doing and I can see rewriting sentences to put line breaks at the 80 char point tripping us up in the future.

@jayaddison
Copy link
Collaborator Author

I think linting of documentation can make sense, so in a way I'm reluctant to exclude the docs dir entirely -- but in practice I haven't been able to configure individual rules/checks to be disabled, so.. perhaps excluding the docs dir is indeed the path to follow here.

@jayaddison
Copy link
Collaborator Author

(rewriting the docs to fit within this rule requirement might be OK, but I sense that sooner or later we'd run into a different lint error that we'd want to ignore - and if we don't have an effective way to deactivate them selectively, then that could become a bit of an annoyance)

@jayaddison
Copy link
Collaborator Author

It turns out that one of the attempts I'd made to configure markdownlint using a .markdownlint.json config file was flawed: I had set the line_length suboption of MD013 (line length checks) to false -- hoping that I could selectively disable it without affecting code-block line length checking, section heading line lengths, etc.

It seems that that option does not support false as a boolean value to disable it - it seems to be integer-only. So I've somewhat-arbitrarily set it to 800 (10 * 80), which is larger than our longest documentation line (~680 at the moment) with some headroom.

I think that's the least-bad solution I'm aware of at the moment. See 9066da3 for the relevant commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants