-
Notifications
You must be signed in to change notification settings - Fork 3.5k
chore: lint markdown new-features.md #15149 #15186
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
Conversation
Joibel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need new features not to make it into the repo with these failures too.
39595ae to
cac9a32
Compare
| .PHONY: features-validate | ||
| features-validate: hack/featuregen/featuregen | ||
| # Validate all pending feature documentation files | ||
| $< validate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think people who're developing a new feature would expect to be able to run make features-validate and have it tell them if it would fail linting - but currently this won't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about the same, but I see a bit of a race condition here because in order to run the mdcheck we need to actually have a file and not print to stdout. This makes the features-validate very similar to features-update one. But if we merge those it means that the user running features-update in order to run correctly needs to know the VERSION to set, and I think it is not what we want.
Another option I thought about is to change the features-validate target to temporarily write into a file somewhere, mdcheck and remove the file. In this way it is "okey" if the generated output does not have the correct version but the default one (latest).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really mind how we achieve this, the validate target's only job is to validate the markdown - and fixing it so VERSION doesn't matter is fine. Be as brutal as needed. If it effectively runs update, that's also fine - perhaps we can avoid a temp file by piping the output to markdown lint?
97b6c85 to
0359310
Compare
0359310 to
c8d050d
Compare
Reference argoproj#15149 Currently we do not lint the new-features.md file because it fails straight during a release. This commit makes the new-features.md compliant with the markdownlint tool and it adds linting check at the right time and not during a CI release. Signed-off-by: Gianluca Arbezzano <[email protected]>
c8d050d to
0bd9adb
Compare
Joibel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks
Reference #15149
Motivation
Currently we do not lint the new-features.md file because it fails
straight during a release.
Modifications
This pR makes the new-features.md compliant with the markdownlint
tool and it adds linting check at the right time and not during a CI
release.
I had to fix a few
.features/pendingfiles that could not be combined into anew-features.md without making the linter to go crazy.
And then I added markdownlint in a few more places in the Makefile.
Verification
make features-updateshould work with no issue and lint the generated file.Documentation