-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Generate e-books for every release and PR #874
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
Apparently, draw.io spoils its SVG 1.1 output images with some HTML tags (invalid within the 1.1 standard). These are then rendered ad black blocks by the svgexport NPM package (required by gitbook to export the book). These SVGs were re-exported using Inkscape 1.2 to make svgexport happy. The package does, however, crop source SVGs slightly from each side for whatever reason. This remains to be resolved.
There are still some code blocks which do not fit the A5 format. Let's make A4-sized PDFs the default size. The A5 PDF may still be the oly format suitable for e-book readers, though, as MOBI and EPUB mess code blocks expected to be written in a monotype font up. Bitmap images are still somewhat oversized in the A4 format, though. A CSS rule similar to the following one may be needed in a CSS file at the right location in the repository: @media print { img { object-fit: scale-down; } }
Synchronization with original repository.
This reverts commit 9557d17.
|
Isn't docker over-engineering for this? And why still use gitbook? |
The whole repository is a gitbook project and the Docker-based solution is the original way it was supposed to be exported. What I have done is merely to finally integrate existing workflow for e-book exporting with GitHub. Personally, I would not be opposed to have the project migrated to something maintained, but that is certainly not a question for me - I am not the author. And as it would require re-writing code snippets and would eventually break compatibility with the on-line version hosted at gitbook's cloud, this is a less invasive method for now, I guess. |
|
Why would we need to rewrite code snippets? The only thing I changed in my original PR was the x86 marker, that too using sed only when exporting. Aside from that nothing was changed and thus compatibility with gitbook cloud was still there. We don't really need a Makefile if we are providing generated ebooks. Those who want to build locally can easily follow the workflow. |
|
Sorry, this is not how I read the comments. |
|
@kyselejsyrecek First of all thank you for PR!
Understandable and thank you for this findings. Can I ask you just to create a separate issue for your findings (including problems with SVG) to not lost them when I'll have a bit more time to return to the exporting issues and improvements.
It can be, but I suppose @kyselejsyrecek just continued based on what already was done. So yes, maybe it was over-engineering from the very beginning, but for now, considering limited time I'd say it is pretty OK to just improve what we have. In future, when I'll finish to update the content itself, I'll take a look what we can do here. I am not strictly for gitbook and maybe we'll move from it completely, not only for exporting. But for now as I said to improve what we already have is pretty of for me.
This I'd leave, it may simplify debugging of the issues mentioned above. |
| # To also create a release for each push to the main branch, uncomment the following 2 lines: | ||
| # branches: | ||
| # - master | ||
| pull_request: |
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.
For this moment I'd also comment this as probably to generate export for each PR while I am rewriting content and it is in the "middle" state is not the best time.
@kyselejsyrecek what do you think?
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.
Might be. I knew the generated e-book or a PDF output might come handy sometimes just to be able to briefly check if a change, yours or others', looks rendered the way you would expect it to. It runs in parallel with the code check and the file retention period is configurable with the retention-days attribute. The output is re-generated each time the code in the pull request is updated. It does not create a new release, of course, just attaches a link to the generated files to the PR. That part might be confusing. UPDATE: I'll separate these jobs anyway.
But the point really was to just provide this possibility if wanted, not to insist on having it merged. I am quite new to the GitHub Workflow API but I guess that this section may be moved to another YAML file and then, from the GitHub GUI, one may disable the separate workflow for however long they chose. Or it could be completely removed if you choose. I am guessing there is no way to make it work for pull requests from the repository forks at this moment on GitHub, so it might actually turn out to be worthless anyway.
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.
After day of thinking I am OK with this. Let's leave it as is but let's maybe make for now data retention shorter, like week or two.
In future it probably will be more comfortable to generate PDF after each commit to markdown file and keep the pdf and other in git tree 🤔. We will see. In general I am pretty happy with the current state what you did.
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.
Alright. The data retention period was shortened to 7 days.
|
I've leaved some comments and questions but very good job in general! @kyselejsyrecek could you also please rebase to master, the |
Synchronization with the original repository
Post-review fixes to GitHub workflow from PR to the original repository
|
@0xAX, thank you. My master branch is now synchronized with the original repository. The fixes have just been merged to my master and this PR has been updated in consequence. Within these changes, I have separated jobs for e-book release and automated e-book attachments to PRs. In the meantime, I have realized there is actually a workaround at hand for these incompatible SVGs. Since the svgexport package is merely converting SVGs to PNGs with some pre-defined resolution (can be verified with inspection of the .MOBI file in Calibre) and unlike svgexport, Inkscape is capable of doing it right, I guess that these SVGs can be converted automatically to PDFs using Inkscape CLI. I already have a sort-of working solution but another PR should probably come tomorrow. I still think @siddhpant's solution might be the better way forward if all issues with the migration are resolved. It is just that I feel uneasy leaving some of my work undone after having it started. The original solution which had been taking advantage of honkit may differ from this PR in the following aspects:
|
If it is such a big problem with SVGs let's try to use PNG? I started to move to SVGs because I can edit them, but nothing will stop me from the continue to draw SVGs and keep them in the repo but in the same time to convert them to PNG and use them in markdown. What do you think? Will it simplify the job? If yes, I'll try to find time on this week to convert them and push to your PR to see the exported docs if you don't mind?
Yes, sure, it is understandable, and as I suggested above let's just create a separate issue to track these issues and we'll see in future how to better fix them, maybe we'll find an easier solution than githook. |
Change e-book retention period in PRs to 7 days
|
@0xAX: The pull request has just been updated to reflect code review fixes and the shortened retention period for pull requests. As for the SVGs, I do not really think that moving from them altogether would be a good idea. And while it does take some time for this workflow based on gitbook to process SVGs form draw.io correctly, I would not consider leaving two copies of the same image a good practice. Afterall, the SVG is the source code and the workflow/action/pipeline or whatewer one wants to call it is a the build process responsible for any data conversion. That should probably only be done during build. I can imagine situations where pre-exported PNGs would not be good enough for some sort of compiled output. What could help, on the other hand, is update of the Docker container. I believe it is hosted in your Docker repository of some sort (I don!t really have much experience with Docker). If you took the initialization commands from the Docker file, especially after when the Inkscape-based SVG conversion to PNG is merged, I imagine that could speed the process a little bit. |
Not really, it was not me who introduced this image. |
|
@kyselejsyrecek as I said I am more-less OK with the current state and the rest of issues I will try to address after content actualization. Thank you one more time for your efforts, merging. |
Oh, sorry. It was my bad assumption then. Thank you for the review and merging the pull request. |
Description
Changes proposed in this pull request:
Inspired by @siddhpant's solution form #837 (https://github.com/siddhpant/linux-insides/releases), it just uses the original gitbook-based solution.
I am quite confident that the pipeline could be improved:
lrx0014/gitbook:3.2.3Docker image updated (so that it does not have to be modified upon each invocation) andI would consider these as next steps or nice to have. Would be great still if anyone interested has the time and abilities to do that on their own, as I almost certainly would not find the time for it.
There are still some code blocks which do not fit the A5 format. For that I am making A4-sized PDFs the default size. The A5 PDF may still be the only format suitable for e-book readers, though, as MOBI and EPUB mess code blocks expected to be written in a monotype font up.
Demo
See kyselejsyrecek#5 (comment) for an example of a pull request for which e-books were generated by the "action" as GitHub calls it.
A new release was generated by action https://github.com/kyselejsyrecek/linux-insides/actions/runs/17412188217 which was triggered by pusing the `v0.1' tag into my fork of the repository. See https://github.com/kyselejsyrecek/linux-insides/releases/tag/v0.1 or https://github.com/kyselejsyrecek/linux-insides/releases.
Issues
Related issues