-
Notifications
You must be signed in to change notification settings - Fork 5
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
Parse and serialize Step resources as WorkloadResources #140
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #140 +/- ##
=======================================
Coverage 92.09% 92.10%
=======================================
Files 65 65
Lines 2088 2114 +26
Branches 356 360 +4
=======================================
+ Hits 1923 1947 +24
- Misses 81 82 +1
- Partials 84 85 +1 ☔ View full report in Codecov by Sentry. |
c98c70d
to
7c76fbf
Compare
7c76fbf
to
bc5b464
Compare
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.
Looks legit!
Maybe we should confirm that the devices handling (only adding to the default preset) is ok before merging?
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.
👍 for readability – and also for not rewriting history, so seeing just the changes since the previous review was possible.
0c53324
to
b6c6ae1
Compare
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.
Alas, poor functools! I knew him, Horatio
b6c6ae1
to
4bc08ab
Compare
@akx takes a look of this. |
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 is some hairiness here, but the hair needs to go somewhere.
Step will now always populate its
resources
attribute with aWorkloadResources
object, which will additionally always populate its full structure down to the value fields so you can query it easily. Empty resources will serialise back into nothing when outputting back into YAML.