-
Notifications
You must be signed in to change notification settings - Fork 0
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
Added generator logic #10
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10 +/- ##
===========================================
+ Coverage 82.32% 92.56% +10.24%
===========================================
Files 4 4
Lines 198 323 +125
Branches 19 43 +24
===========================================
+ Hits 163 299 +136
+ Misses 35 22 -13
- Partials 0 2 +2 ☔ View full report in Codecov by Sentry. |
0e313f7
to
2c6eeae
Compare
2c6eeae
to
dbfb972
Compare
Hi @josephlewis42, thank you very much, I will review this tomorrow, I missed the notification in the avalanche I got when coming back from holidays, sorry for that! |
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.
See few inline comments
I've created a ZIM of lua~5.4 and cpp. I will open separate issues to discuss what I've found there (nothing which is a blocker to merge this code, more stuff to be discussed / fixed before releasing a 0.1.0 version)
Regarding the time it takes to complete, I'm not that surprised. cpp tooks 15 minutes to complete, which is OK. We are used to scraper taking hours or even sometime days to complete, so this is not a concern from my PoV. But still good to mention and try to enhance, the faster the scraper can be the better.
Regarding known limitations, I suggest we open one issue per limitation so that we can discuss separately how to address them (or even decide to not address them ^^). I agree we do not need to fix them before the MVP.
dbfb972
to
d91283a
Compare
d91283a
to
353798e
Compare
Adds an MVP version of the generator: #1. With this, we get pretty nice support for most DevDocs items.
Known limitations:
third_party
assets are checked in and will need to be manually refreshed. Sadly, this info is only in the GitHub repo, but not the generated content ondevdocs.io
so there's a small window where it might become out of sync.ansible
which have very large number of entries in their navbars which causes Jinja to choke when instantiating and escaping tons of objects.libzim
compression is also slow: User-defined compression level libzim#544, we need some compression because images are base64 encoded and the navbars are repetitive, but doing so at the highest level is spending a bunch of time trying to further compress things with already high information density (e.g. JPEGs).Once merged, good proof of concepts for this would be: