-
-
Notifications
You must be signed in to change notification settings - Fork 19
Add utility to fetch and prepare ZIM illustration #260
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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #260 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 39 40 +1
Lines 2455 2480 +25
Branches 331 334 +3
=========================================
+ Hits 2455 2480 +25 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
d23a061
to
d597620
Compare
788e4ae
to
baad1c3
Compare
d597620
to
dab6ba2
Compare
Rebased on main for simplicity of review |
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 it's great.
I would have preferred not to extend the Illustration Metadata constructor. That seems unnecessary and it shifts the (unwritten) spirit of that API towards a more logic-intensive approach than I expected it to be.
I like that it's designed to receive proper values with safety-net-like checks that prevents breaking our convention. Sending inappropriate data and store a transformed version feels off to me.
Parsing, downloading, converting, optimizing, it's a lot. I expect shit to happen and I find cleaner to do that as a separate step then put the successful result into the metadata.
Let me know what you think
I don't have strong PoV on this tbh, this was more a suggestion you made as far as I remember. I see benefits and arguments to both approaches, so let's remove this for now. |
08ead32
to
75fb60b
Compare
Fix #254
Nota : code has been adapted from libretexts/warc2zim version to also optimize the image, hence fixing the issue found recently (openzim/warc2zim#448)