-
Notifications
You must be signed in to change notification settings - Fork 363
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
Unify yaml serialization dependencies #266
Conversation
Codecov Report
@@ Coverage Diff @@
## master #266 +/- ##
==========================================
+ Coverage 53.76% 53.83% +0.07%
==========================================
Files 18 18
Lines 863 860 -3
==========================================
- Hits 464 463 -1
+ Misses 342 341 -1
+ Partials 57 56 -1
Continue to review full report at Codecov.
|
Standardize on sigs.k8s.io/yaml. Bump k8s.io/apimachinery depenceny as it used to depend on module github.com/ghodss/yaml Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>
Hmm interesting enough this test was failing locally at some point (like below) but they're good now.
|
/hold cancel |
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.
Nice.
@@ -86,7 +84,6 @@ func LoadPluginFileFromFS(pluginsDir, pluginName string) (index.Plugin, error) { | |||
|
|||
// ReadPluginFile loads a file from the FS. When plugin file not found, it | |||
// returns an error that can be checked with os.IsNotExist. | |||
// TODO(lbb): Add object verification |
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.
🎉
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.
nice
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 |
Done. |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahmetb, corneliusweig 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 |
/lgtm |
github.com/ghodss/yaml
Fixes #265.
/hold
Currently there are some test issues. Looks like the parser is actually doing
some unnecessary defaulting on metav1.ObjectMeta fields. This is making tests
difficult.