-
Notifications
You must be signed in to change notification settings - Fork 68
🌱 Add yamlfmt to ensure consistent testdata yaml file formatting #2284
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
🌱 Add yamlfmt to ensure consistent testdata yaml file formatting #2284
Conversation
Signed-off-by: Per G. da Silva <pegoncal@redhat.com>
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
Pull Request Overview
This PR adds the yamlfmt tool to ensure consistent YAML formatting across testdata files. The tool is integrated into the project's build system and applied to existing testdata files to standardize their formatting.
- Adds
yamlfmttool via bingo dependency management - Integrates
yamlfmtinto themake fmttarget to automatically format testdata YAML files - Applies consistent formatting to existing testdata YAML files (removes leading
---markers, unwraps multi-line descriptions, normalizes spacing)
Reviewed Changes
Copilot reviewed 12 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Makefile | Updated fmt target to include yamlfmt execution on testdata directory |
| .bingo/yamlfmt.mod | Added yamlfmt module dependency configuration |
| .bingo/yamlfmt.sum | Added checksums for yamlfmt and its dependencies |
| .bingo/variables.env | Added YAMLFMT environment variable |
| .bingo/Variables.mk | Added yamlfmt build target |
| testdata/images/catalogs/test-catalog/v2/configs/catalog.yaml | Removed leading --- marker |
| testdata/images/catalogs/test-catalog/v1/configs/catalog.yaml | Removed leading --- marker and trailing blank line |
| testdata/images/bundles/webhook-operator/v0.0.1/manifests/webhook.operators.coreos.io_webhooktests.yaml | Unwrapped multi-line description fields to single lines |
| testdata/images/bundles/test-operator/v1.2.0/manifests/olm.operatorframework.com_olme2etest.yaml | Removed leading --- marker |
| testdata/images/bundles/test-operator/v1.2.0/manifests/bundle.configmap.yaml | Unwrapped multi-line annotation value to single line and added trailing blank line |
| testdata/images/bundles/test-operator/v1.0.0/manifests/olm.operatorframework.com_olme2etest.yaml | Removed leading --- marker |
| testdata/images/bundles/test-operator/v1.0.0/manifests/bundle.configmap.yaml | Unwrapped multi-line annotation value to single line and added trailing blank line |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2284 +/- ##
==========================================
+ Coverage 70.32% 71.19% +0.87%
==========================================
Files 90 90
Lines 8794 6995 -1799
==========================================
- Hits 6184 4980 -1204
+ Misses 2196 1601 -595
Partials 414 414
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| fmt: #EXHELP Formats code | ||
| fmt: $(YAMLFMT) #EXHELP Formats code | ||
| go fmt ./... | ||
| $(YAMLFMT) testdata |
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.
But I think it won’t be caught in the CI.
I mean, will it actually fail after this PR if the code isn’t formatted?
I think we should make sure that it’s being checked as well.
Just to share — in Kubebuilder we use this: ( to lint yaml )
👉 https://github.com/kubernetes-sigs/kubebuilder/blob/master/Makefile#L128-L131
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.
it should be caught by the verify target since it calls the fmt target as well. So, changes ought to get caught by CI.
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.
right?
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.
Dropping the formatting commit to test
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.
Signed-off-by: Per G. da Silva <pegoncal@redhat.com>
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.
Pull Request Overview
Copilot reviewed 12 out of 17 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
camilamacedo86
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.
/lgtm
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, tmshort 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 |
3a6afed
into
operator-framework:main
Description
yamlfmttool ref to bingomake fmttarget to run yamlfmt against the testdata directoryReviewer Checklist