Skip to content
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

Fix generating the PDF and improve its formatting #393

Merged
merged 3 commits into from
Dec 4, 2024
Merged

Fix generating the PDF and improve its formatting #393

merged 3 commits into from
Dec 4, 2024

Conversation

vedran-kasalica
Copy link
Member

@vedran-kasalica vedran-kasalica commented Dec 3, 2024

Changes in this PR

I updated the page.yml GitHub action to generate PDFs correctly, namely:

  • I migrated from docsify-pdf-converter (outdated) to docker-docsify-pdf. The new PDF generator creates a cleaner PDF that includes the TOC by default.
  • GitHub action now adds the PDF as an artifact (see the fig. below)
  • Pushing to Zenodo is conditional (it will not be done when the workflow is triggered manually, in that case, it only generates the PDF, for testing purposes). This could be updated to push to sanbox.zenodo.org, but that should be a separate issue.

Smaller changes:

  • I added a header to the technology_overview page as it was missing (noticeable when generating a PDF).
  • I also included a cover page for the PDF. I added it under Figures not to clutter the root dir, but this can be changed.

The PR resolves issues:
#270 (PDF is generated, Upload to Zenodo should be tested at the release)
#368 (TOC is now included
#369 (while the formatting is not perfect, it is much better, e.g., each section starts on a separate page, table formatting does not break, etc.)

Screenshot 2024-12-04 at 10 52 47

Checklist

SIGNIFICANT changes / additions, e.g. new chapters

  • I discussed my contribution in an issue and took into account feedback.

ALL contributions

  • I previewed my changes locally using e.g. python3 -m http.server 4000 and confirmed they work correctly.
  • I checked for broken links, e.g. using the link checker GitHub Action workflow, or locally by using docker run --init -it -v `pwd`:/docs lycheeverse/lychee /docs --config=docs/lychee.toml, at least for the files I changed.
  • My name was added to the CITATION.cff file.

@vedran-kasalica vedran-kasalica changed the title Include PDF cover and add a chapter title Fix generating the PDF and improve its formatting Dec 4, 2024
Copy link
Member

@egpbos egpbos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great PR, thank you for fixing this process and making the output much better as well! I have just one suggestion below, I'll just go ahead and commit that and then I think we can merge that. I have a few follow-up issues that I will create as well:

  • Add headers to all chapters (you added one to the Technology overview page, others should have it as well).
  • The PDF should have the version somewhere, preferably on the cover, but colofon or something is fine too.
  • Your point: "Pushing to Zenodo is conditional (it will not be done when the workflow is triggered manually, in that case, it only generates the PDF, for testing purposes). This could be updated to push to sanbox.zenodo.org, but that should be a separate issue."

release:
types: [published]

jobs:
generate:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great that you also updated the action versions (also for setup-node below)!

technology/technology_overview.md Outdated Show resolved Hide resolved
@egpbos egpbos merged commit a459278 into main Dec 4, 2024
2 checks passed
@egpbos egpbos deleted the pdf-fix branch December 4, 2024 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve the PDF layout Add the table of contents to the generated PDF
2 participants