-
Notifications
You must be signed in to change notification settings - Fork 126
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
feat: add aggregation
field in KeptnMetric
#1780
feat: add aggregation
field in KeptnMetric
#1780
Conversation
Signed-off-by: Rakshit Gondwal <[email protected]>
✅ Deploy Preview for keptn-lifecycle-toolkit ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
aggregation
field in `KeptnMetricaggregation
field in KeptnMetric
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1780 +/- ##
==========================================
+ Coverage 63.57% 63.67% +0.10%
==========================================
Files 143 143
Lines 10937 10957 +20
==========================================
+ Hits 6953 6977 +24
+ Misses 3722 3719 -3
+ Partials 262 261 -1
... and 1 file with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. |
Signed-off-by: Rakshit Gondwal <[email protected]>
Signed-off-by: Rakshit Gondwal <[email protected]>
Signed-off-by: Rakshit Gondwal <[email protected]>
Signed-off-by: Rakshit Gondwal <[email protected]>
Implementation wise this looks good! Can you please also update the CRD docs by executing the following command:
Note: It seems like currently the workflow for checking whether the CRD docs are up to date is currently not executed. I created an issue for that in #1817 (this is of course out of scope for this PR) |
Signed-off-by: Rakshit Gondwal <[email protected]>
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
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, great work!
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.
Well done 👏
The failing html tests are not due to this PR :) |
Thank you for the merge! |
Fixes #1779
Changes made:
KeptnMetric
struct.range.step
is not defined andrange.aggregation
is defined, afield.Required
error would be shown.range.aggregation
is not defined andrange.step
is defined, afield.Required
error would be shown.