-
Notifications
You must be signed in to change notification settings - Fork 53
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
Hidden manifests implementation (new) #1699
Conversation
Minor: also update old test that is now failing on python3.13+
This is not a solid way to set the default value that is now migrated to the collect-manifest job
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1699 +/- ##
==========================================
+ Coverage 49.01% 49.12% +0.10%
==========================================
Files 372 372
Lines 40348 40338 -10
Branches 6817 6809 -8
==========================================
+ Hits 19777 19816 +39
+ Misses 19849 19799 -50
- Partials 722 723 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Minor: Move default value calculation to the manifest unit
cffcfed
to
0d3e1b9
Compare
0d3e1b9
to
1e5ef3c
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.
Good job, Max!
The PR looks good to me. The hidden manifest implementation is what we've discussed previously and the renamings here and there look coherent.
Also, the metabox tests seem to cover the functionality properly.
I've left a small comment if you fancy to change it, but I leve it to you.
Description
We need a “hidden” manifest type to be able to include/exclude jobs runs in the lab without impact normal end user experience:
Hidden = not available for general consumption, not shown in the Manifest entries screen
hidden keyword raises questions, so need an additional hidden_reason field to explain why this manifest is not shown to the user (e.g. “No OBEX server testing required in the HW Cert lab”)
These manifest entries must be false (i.e. not set) for submissions made to issue a Certificate (Cert issuance process must include this check, this is a critical violation because it can lead to important things not being tested)
Hidden manifest IDs must start with _ (underscore). This is for easier reading/parsing/debugging, and must have a hidden-reason field to explain why they were introduced.
Resolved issues
Fixes: CHECKBOX-1698
Documentation
New field documented in reference
Tests
Tested via metabox + unittest