thewca / wca-documents

Official Documents of the World Cube Association.
https://www.worldcubeassociation.org/documents
20 stars 37 forks source link

Update the build pipeline #386

Closed dmint789 closed 2 months ago

dmint789 commented 3 months ago

This is a big PR, but this was a very important update for us to make. It should improve the developer experience greatly when it comes to building the PDFs to see a preview of the documents, which was previously only really easy to do by creating a PR and waiting for the Github pipeline to do it, which always took a while and made it harder to iterate with minor adjustments. This PR does the following:

dmint789 commented 3 months ago

@nsilvestri could you provide an example of an unwanted page break due to the spacing between lines? For me the spacing looks the same as it does in production rn.

dmint789 commented 3 months ago

Staff crash course fixed

nsilvestri commented 3 months ago

@dmint789 the page breaks caused by spacing are not "unwanted", just slightly different. 1-2 lines will be pushed to the next page, or vice versa. Very minor stylistic thing, just noting it as an inconsistency.

dmint789 commented 3 months ago

Fixed the bullet point issue in the competitor tutorial

dmint789 commented 3 months ago

Okay, now I need to update the Github actions. I think I've fixed all of the issues you found.

dmint789 commented 3 months ago

Found another issue: the counters reset when there is content between two items (see the competition templates doc for an example). Currently working on fixing this.

dmint789 commented 3 months ago

Fixed

dmint789 commented 3 months ago

I have personally gone through every single markdown document in the repository, fixed all alignment issues and made sure they all look almost exactly the same as the original docs rendered with wkhtmltopdf. The PR is now ready for review.

@nsilvestri could you please review the changes in the documents and edudoc folders? It's almost exclusively alignment fixes and changes from pure HTML to markdown/pandoc syntax in some of our old docs. My expectation is that you won't find any issues, as I tried being as thorough as possible, but it's good to have another set of eyes on the changes.

@thewca/software-team could you please review the changes in the root of the repo? Namely, the changes to the assets and bin folders, the README and my newly-created Dockerfile.

I will also review all of the aforementioned changes myself for good measure.

dmint789 commented 3 months ago

@thewca/software-team also, I'll need someone to override the PR build test, cause it's failing due to not being up to date with these changes

dmint789 commented 3 months ago

@gregorbg is this good to be merged now?

dmint789 commented 2 months ago

Looks like all issues are resolved now

dmint789 commented 2 months ago

Oh, maybe it's the apt commands. Also, does the build pipeline use the setup files from master or from this branch I'm trying to merge? I assumed the CI error would never go away until this PR is merged.

dmint789 commented 2 months ago

@thewca/board can you please approve? This probably requires Board approval, because I had to edit many of the documents themselves, but the only real change is the last point in the description of this PR, but it doesn't even affect the rendered file, since the numbers themselves are ignored. I just wanted to fix the numbering in the Bylaws Markdown. I have personally checked the rendering of every single document in this repo, and everything has remained consistent, bar some very minor alignment here and there.

dmint789 commented 2 months ago

@gregorbg Looks like I've answered my own question, as the PR build test has a different output after my commit now. I'll try to fix it, looks like the repos used by the github action are different from the ubuntu image repos.

dmint789 commented 2 months ago

@gregorbg according to this, package version locking is not possible on ubuntu. I'd just remove the pandoc and weasyprint package versions and call it a day, unless you know of a way to get a specific version of these packages from apt.

dmint789 commented 2 months ago

@gregorbg the checks are finally passing. Can you please re-approve, cause the new commits made your approval stale. Maybe we can revisit package version locking in another issue and PR, cause this is currently blocking two of our projects.