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: fix docs CI workflow; build in MRs; build and deploy on master #2504

Merged
merged 2 commits into from
May 24, 2024

Conversation

iamorphen
Copy link
Contributor

@iamorphen iamorphen commented May 22, 2024

Introduction

The Build and deploy GH Pages GitHub Actions workflow defined in docs.yml is designed to run on pushes to master. However, we don't check that the docs build correctly before allowing MRs that change the docs to land. This sometimes results in failures in the workflow on master.

The change here introduces another job in the workflow, one that runs only in merge requests and performs a docs build. The previous job is adapted to only run on pushes to master, but it still performs a build and deployment.

The change here essentially adapts the usage of a newer version of the Action to the zola repository.

Testing

Without this change, on master, run

cd docs
zola build

You can reproduce the CI failure. With this change, zola build succeeds locally and in CI.

@iamorphen iamorphen marked this pull request as ready for review May 22, 2024 08:01
@iamorphen
Copy link
Contributor Author

@Keats I meant to test this in draft, but since I modified the workflow, looks like I need to get approval before it will run. The new job run only in merge requests doesn't require any secrets. (I do expect a workflow failure given the docs issue on master currently; will fix that after I confirm that the workflow modification works.)

@iamorphen
Copy link
Contributor Author

I guess to catalog zola themes we extract their README. The issue with the pico theme was that the README contained a (zola) relative link, but the necessary content is not present here. It seems the link in the pico README wants to point here, so I just used the absolute link.

@iamorphen
Copy link
Contributor Author

Also, I made the pico fix in another commit because I wasn't sure if changing the commit hash of the workflow modification would require me to get reapproval on the workflow. Not sure about the merge strategy here (squash, rebase, etc.), so just a heads up.

@Keats
Copy link
Collaborator

Keats commented May 23, 2024

The theme directory is auto-generated from the themes repo so that change will be removed next time an update is made. We should just ignore link checking in the themes section. I believe there is an option for that but can't check atm

@iamorphen
Copy link
Contributor Author

Ah, thanks. I'll poke around tomorrow and see what I can find.

@iamorphen iamorphen force-pushed the fix/orphen/fix-docs-workflow branch from e3c0043 to 57aee32 Compare May 24, 2024 02:18
@iamorphen
Copy link
Contributor Author

For posterity, my understanding is that this workflow updates the themes in a peer repository, and that workflow calls this which updates the content of docs/content/themes/ here and creates an MR. I found link_checker configuration docs here and so applied them to docs/config.toml.

Now in CI we survive broken links.

Warning: Broken relative link `@/notes/_index.md` in themes/pico/index.md

@Keats Keats merged commit 3d0749d into getzola:master May 24, 2024
7 checks passed
@Keats
Copy link
Collaborator

Keats commented May 24, 2024

Thanks!

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.

None yet

2 participants