-
Notifications
You must be signed in to change notification settings - Fork 780
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: support source field in Constraints #2552
fix: support source field in Constraints #2552
Conversation
Signed-off-by: davis-haba <davishaba@google.com>
Signed-off-by: davis-haba <davishaba@google.com>
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #2552 +/- ##
==========================================
- Coverage 53.46% 53.11% -0.35%
==========================================
Files 120 120
Lines 10634 10560 -74
==========================================
- Hits 5685 5609 -76
- Misses 4512 4515 +3
+ Partials 437 436 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 5 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
Signed-off-by: davis-haba <davishaba@google.com>
Signed-off-by: davis-haba <davishaba@google.com>
Signed-off-by: davis-haba <davishaba@google.com>
Overall approach LGTM, though I think we can make the bash file much simpler (IMO that file should be nearly trivial) |
Signed-off-by: davis-haba <davishaba@google.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.
Couple of comments, but definitely on-track.
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
Signed-off-by: davis-haba <davishaba@google.com>
Signed-off-by: davis-haba <davishaba@google.com> exclude matchcrd_constant.go from linter Signed-off-by: davis-haba <davishaba@google.com>
79f224f
to
daaa257
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.
Thank you! LGTM
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
@davis-haba good to merge? |
Signed-off-by: davis-haba <davishaba@google.com> Co-authored-by: Rita Zhang <rita.z.zhang@gmail.com>
Signed-off-by: davis-haba <davishaba@google.com> Co-authored-by: Rita Zhang <rita.z.zhang@gmail.com> Signed-off-by: Xander Grzywinski <xandergr@microsoft.com>
What this PR does / why we need it:
This fixes an issue where the
match.source
field was not recognized in Constraints. It also adds code to auto generate theJSONSchemaProps
for theMatch
type, to prevent this manner of error in the future.An e2e test for the source field on Constraints is added.
Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when the PR gets merged):Fixes #2485