-
Notifications
You must be signed in to change notification settings - Fork 10
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
validate prescriptions with pre-commit hook using adviser #38951
validate prescriptions with pre-commit hook using adviser #38951
Conversation
/test all |
a37387f
to
e7c60fc
Compare
/unhold |
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.
just one pre-commit thing to fix
.pre-commit-config.yaml
Outdated
- id: validate-prescriptions | ||
files: prescriptions/ | ||
exclude: prescriptions/_prescription_metadata.yaml |
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.
yamllint is complaining: error wrong indentation: expected 2 but found 4 (indentation)
e7c60fc
to
4d5c139
Compare
Once we have a proper release of adviser we'll use that
of course I had to forget the other pre-commit hooks :/ |
/lgtm thanks! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: codificat The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Related Issues and Dependencies
fix #8
thoth-station/adviser#2402
This introduces a breaking change
This should yield a new module release
This Pull Request implements
Add a pre-commit hooks to validate prescriptions.
In conjonction with --from-ref / --to-ref, this should only validates files touched in a PR
This on my fork of thoth-station/adviser because it needs thoth-station/adviser#2411 and #2408 to be merged first.
By the way, we really need operate-first/apps#2462 on this repo. It would allow us to implement pre-commit hook caching instead of installing every single time, which would save a lot of prow-time. (well, once it comes back up...)
/hold