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

Fix .md docs #12378

Merged
merged 3 commits into from
Oct 13, 2023
Merged

Fix .md docs #12378

merged 3 commits into from
Oct 13, 2023

Conversation

simondeziel
Copy link
Member

No description provided.

@simondeziel
Copy link
Member Author

Looks like the error format changed with mdl 0.12, causing our error filtering/ignore list to not work anymore.

@tomponline
Copy link
Member

@ru-fu please could you take a look at this?

@ru-fu
Copy link
Contributor

ru-fu commented Oct 13, 2023

I just added a commit that should fix it.
Feel free to prettify it though, Simon. ;)

@ru-fu
Copy link
Contributor

ru-fu commented Oct 13, 2023

Here's the action run with my fix, but without yours: https://github.com/ru-fu/lxd/actions/runs/6506990288/job/17673588099
(Failing as expected.)

@ru-fu
Copy link
Contributor

ru-fu commented Oct 13, 2023

doc/doc-lint: remove unneeded "|| true"

This one took me forever to figure out, and I'd love to see a good solution for it. ;)
The issue is that after filtering out the expected errors, we're left with an empty list. However, that means that grep returns an error and the script fails, it never gets to the check ...

@simondeziel
Copy link
Member Author

doc/doc-lint: remove unneeded "|| true"

This one took me forever to figure out, and I'd love to see a good solution for it. ;) The issue is that after filtering out the expected errors, we're left with an empty list. However, that means that grep returns an error and the script fails, it never gets to the check ...

Well, the problem I saw with it was that it was outside of the subshell "$()" || true and I'm used to seeing it inside. Now that I dropped it, grep's error is failing the run so it wasn't uneeded (I was wrong).

I cherry-picked your fix (thanks!) and I'm now trying to iterate on solution that'd provide more context in case of error. I was thinking of using something like diff.

@ru-fu
Copy link
Contributor

ru-fu commented Oct 13, 2023

I cherry-picked your fix (thanks!) and I'm now trying to iterate on solution that'd provide more context in case of error. I was thinking of using something like diff.

But for failures, we print the filtered_errors variable - that should be sufficient?

simondeziel and others added 3 commits October 13, 2023 09:50
Signed-off-by: Simon Deziel <simon.deziel@canonical.com>
Signed-off-by: Simon Deziel <simon.deziel@canonical.com>
The markdown linter now outputs a list of rules in addition to the
error (changed in markdownlint/markdownlint#407).

Update the script to get rid of these, and deal with the now
empty list if all errors are filtered.

Signed-off-by: Ruth Fuchss <ruth.fuchss@canonical.com>
Signed-off-by: Simon Deziel <simon.deziel@canonical.com>
@simondeziel simondeziel marked this pull request as ready for review October 13, 2023 14:56
@tomponline tomponline merged commit b54a977 into canonical:main Oct 13, 2023
25 checks passed
@simondeziel simondeziel deleted the doc-check-fix branch October 13, 2023 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Documentation needs updating
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants