-
Notifications
You must be signed in to change notification settings - Fork 35
adding unzipped cdk8 data #1151
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
Conversation
mikemhenry
left a comment
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
mikemhenry
left a comment
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.
Oh can you update the manifest?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1151 +/- ##
==========================================
- Coverage 93.53% 91.89% -1.64%
==========================================
Files 142 142
Lines 10635 10633 -2
==========================================
- Hits 9947 9771 -176
- Misses 688 862 +174
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mikemhenry
left a comment
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
|
No API break detected ✅ |
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.
Last year, the team made a decision to try to gzip all PDB files going forward. Doing so here would be easy / low cost, can we try to do this too?
Can we possibly discuss this more? It runs somewhat against things we previously agreed upon as a team. I can see the reason for this decision, but I suspect there's context to the "floodgates" this migth open if unrestricted. If anything, the minimum here is that the whole team should be aware of what the current file upload standards are (it's something we hit on quite often during Protocol development). |
Here I mean the above-mentioned "gzipping PDB files where we can", approach. |
|
Yes of course -- we are talking about 630K difference: We should probably add some tooling then to make it easy to pack and unpack test data if we want to treat it as a big tarball hosted somewhere else, or even if we decide to keep them in out git repo. |
since @mikemhenry and I decided that 1MB is our cap for file sizes, it's fine for us to commit the cdk8 dataset as-is.
resolves #1078
Developers certificate of origin