Skip to content

Add CI for PDF generation#90

Merged
0xAX merged 1 commit into
0xAX:masterfrom
rcalixte:mk_pdf
Jul 16, 2026
Merged

Add CI for PDF generation#90
0xAX merged 1 commit into
0xAX:masterfrom
rcalixte:mk_pdf

Conversation

@rcalixte

@rcalixte rcalixte commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Description

Add CI to automatically generate PDFs on pushes to master with a manual trigger for creation as well.

A CI job sample run is here the forked repository contains a release asset with the PDF as well.

Changes proposed in this pull request:

Content modifications are to support PDF generation:

  • SVGs were converted to PNGs
  • One newline inserted

Related issues

@rcalixte rcalixte requested review from 0xAX and klaudiagrz as code owners July 6, 2026 19:57
@0xAX

0xAX commented Jul 6, 2026

Copy link
Copy Markdown
Owner

@rcalixte thank you very much! I will take a look tomorrow morning

@0xAX

0xAX commented Jul 7, 2026

Copy link
Copy Markdown
Owner

@rcalixte I tested it and it looks really great! Thank you a lot. I have just two little suggestions that I'll describe in the next comments.

Comment thread .github/workflows/gen-pdf.yaml Outdated
Comment thread scripts/pdf/include.tex

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can we move this and all related scripts below, pandoc configuration and so on to ./scripts/pdf directory or something like this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We should be able to move things around. I'll include this in the update as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This works but it's kind of hacky in that the scripts still expect to be run from the subdirectory while the contents are a different level from the root. I opted for this behavior in the scripts since I think most folks running the scripts locally would do so from the directory itself. We can refactor if you think that assumption isn't fair.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@rcalixte yes, thank you. I think it is pretty ok to me.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I tested, works great.

To avoid the issue you mentioned, can you please just add:

# The pandoc-*.yaml configs and the paths inside them are relative to this
# script's directory, so always run from there regardless of where the
# script was invoked from.
cd "$(dirname "$0")" || exit 1

to the top of the pandoc.sh. It will make it work from both dirs, and from root dir and from the scripts dir.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Great fix! Just when I thought I'd seen it all!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

After discussing with a friend, I opted for pushd/popd over cd but I can revert if you want.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

After discussing with a friend, I opted for pushd/popd over cd but I can revert if you want.

No no, it is cool!

@rcalixte rcalixte force-pushed the mk_pdf branch 3 times, most recently from cc091dc to 9c93155 Compare July 14, 2026 23:16
@0xAX

0xAX commented Jul 15, 2026

Copy link
Copy Markdown
Owner

@rcalixte thank you for the update. In general looks good to me! I will just test it a bit later today/tomorrow and if everything is good lets merge

@0xAX

0xAX commented Jul 15, 2026

Copy link
Copy Markdown
Owner

@rcalixte Only little comment about pandoc.sh, can you please add it and let's merge!

@rcalixte rcalixte force-pushed the mk_pdf branch 2 times, most recently from 2addd38 to dfba97b Compare July 15, 2026 22:17
@0xAX 0xAX self-requested a review July 16, 2026 09:28
@0xAX

0xAX commented Jul 16, 2026

Copy link
Copy Markdown
Owner

@rcalixte thank you a lot for your contribution! Merging.

@0xAX 0xAX merged commit eb7acae into 0xAX:master Jul 16, 2026
1 check passed
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.

2 participants