-
-
Notifications
You must be signed in to change notification settings - Fork 43
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
Document Docker usage in README, issue #228 #230
Conversation
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.
Some changes:
- we do not want to have a
Usage
and aGetting started
sections. I think that it would be much better to rework theGetting started
section to mention Docker there - pulling the Docker image is useless, do not mention it
- the
-m
option is very special in that it creates only one ZIM per language (instead of one ZIM with all requested books). Most users do not want that (they want only one ZIM for their selection of books). Docker instructions should allow to produce a ZIM "available outside of Docker" even when the-m
option is not used. Simply mounting/output
(which is the default WORKDIR of Docker image where temporary files and final ZIM are placed unless I'm mistaken) as a Docker volume is sufficient for that.
I Did the changes. Check if there any mistakes. |
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! Almost good, please fix last points and squash all your commits into one now that we are close to merge.
README.md
Outdated
cd gutenberg | ||
``` | ||
|
||
If you do not already have it on your system, install `hatch` to build the software and manage virtual environments (you might be interested by our detailed [Developer Setup](https://github.com/openzim/_python-bootstrap/wiki/Developer-Setup) as well). |
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.
Please fix link: https://github.com/openzim/_python-bootstrap/blob/main/docs/Developer-Setup.md
(was here before you came as well ...)
edefc80
to
478c6fa
Compare
@benoit74 I squashed all the commits in one. Please check if correct |
Please update your branch to be on-top of latest main commit (and resolve merge conflict, your version is good in fact, it is just a kind of false positive). |
Done, Updated my fork repo :D |
478c6fa
to
0d1614b
Compare
Documented Docker usage in README. Closes #228