-
Notifications
You must be signed in to change notification settings - Fork 448
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: allow specifying components when using edit-validation-sets #5059
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5059 +/- ##
==========================================
- Coverage 94.88% 89.69% -5.19%
==========================================
Files 658 341 -317
Lines 55189 22607 -32582
==========================================
- Hits 52364 20278 -32086
+ Misses 2825 2329 -496 ☔ View full report in Codecov by Sentry. |
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.
Thanks for the PR! I have one comment and one request for updating some tests.
Will you update the dictionaries in tests/legacy/unit/store/v2/test_validation_sets.py
to include components?
Do you happen to know if the store side is ready for this? Testing your branch gives me a store error Issues encountered with validation set: Additional properties are not allowed ('components' was unexpected) at /snaps/0
Sounds good. Once the PR lands, it will be available in the |
76e4f07
to
282df4f
Compare
I think that is around the right time. I'll talk with my team to confirm timelines. |
282df4f
to
4d6840d
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.
Thanks! Once the store changes are completed and we verify it works, we can land this PR.
4d6840d
to
7dba20a
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 good, just added a question about component revision validation.
tox run -m lint
?tox run -e test-py310
? (supported versions:py39
,py310
,py311
,py312
)Hello! This should enable using
edit-validation-sets
with components. Let me know if I'm missing any testing. I don't see any tests that specifically test the allowed format of validation sets, so I don't know if that is something that we need/want to be added.