-
Notifications
You must be signed in to change notification settings - Fork 20
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: verify-manifest/bundle #233
Conversation
Signed-off-by: Guilherme Cassolato <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #233 +/- ##
=======================================
Coverage 61.54% 61.54%
=======================================
Files 2 2
Lines 788 788
=======================================
Hits 485 485
Misses 251 251
Partials 52 52
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
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.
🫣
.PHONY: verify-manifests | ||
verify-manifests: manifests $(YQ) ## Verify manifests update. | ||
git diff -I'^ createdAt:' -I' containerImage:' -I' image:' --exit-code -- ./config ':(exclude)config/authorino/kustomization.yaml' | ||
git diff -I'^ createdAt:' -I'$(OPERATOR_IMAGE_REPO)' -I'$(DEFAULT_AUTHORINO_IMAGE_REPO)' -- ./config ':(exclude)config/authorino/kustomization.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.
removing --exit-code
? it will not fail if there is some change
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.
--exit-code
makes it exist with 1 if there is any difference in the file (other than the ones ignored of course) and 0 otherwise. We want that.
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.
Confused here. When you say "we want that", you mean we want --exit-code
? Then why did you remove it?
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 did!!?? Damn! I thought you were asking to remove it and I argued we need it, but didn’t realise the mistake. Fixing it now.
Thanks @eguzki
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.
No description provided.