-
Notifications
You must be signed in to change notification settings - Fork 125
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
Configure github pages #335
Conversation
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.
Nice! I left some comments, PTAL.
Let's change the "Fixes 326" to "Refs 326" as there are some further steps there.
.github/workflows/linux.yml
Outdated
@@ -52,3 +52,10 @@ jobs: | |||
name: test logs | |||
path: | | |||
build/meson-logs/ | |||
- name: Upload doxygen docs |
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.
I think it's a little strange to upload docs from every test run. I think it'd be more self-contained to buid & upload docs from the pages workflow itself.
But if you prefer to keep it this way, that's OK by me.
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.
the frustrating thing about github workflows is the amount of duplication required. There's no good/easy way (afaik) to compose a single image, then run just the build step on that image (like we do on gitlab with the freedesktop ci-templates). And workflows are separate too, so if we had a separate pages workflow we couldn't even re-use the package lists.
We could move the pages job into a separate jobs
entry and then at least have the pip/apt list as an env var.
But even then it's a lot of duplication - this here is the easiest way to avoid that duplication.
And in case the "upload" term is confusing: this just stores the artifact from the workflow, it doesn't do the actual GitHub pages upload, that's handled in the other workflow which fetches this artifact and pushes it out to pages. And that one only runs on push-to-master.
We could add an extra if:
condition like this - just need to figure out the right magic variables:
- name: Upload doxygen docs
if: ${{ github.event_name == 'pull_request' && github.event.action == 'unassigned' }}
but... meh?
permissions: | ||
contents: read | ||
pages: write | ||
id-token: write |
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.
I'm curious, which step requires this permission?
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.
🤷 This is copied from the example workflows github provides and they all have that section as-is. I'm not 100% why the id-token
needs to be set to write and documentation on it is a bit sparse.
Upload the doxygen output as artifact from the linux build and use that from the pages job where we combine the static website with our newly build HTML docs. The GitHub actions/download-artefact doesn't work across workflows so we use the other popular one that can do this. The rest of the job is basically copy/paste from the "Static HTML" example GitHub provides. To make this useful as drop-in replacement, replace the one fixed link to the API docs a relative one. Signed-off-by: Peter Hutterer <[email protected]>
a236d80
to
f373e99
Compare
Doxygen 1.9.7 breaks our urls, see issue xkbcommon#347. Let's put a check for the doxygen version into our CI build so that if our base distro updates beyond that, the CI fails and we know we have to build doxygen from scratch or update to some other version that's supported. Signed-off-by: Peter Hutterer <[email protected]>
done! right now we're on some earlier version of doxygen, so I put a step in to make sure we don't end up on > 1.9.6 and if we do, the CI should fail. |
@bluetech ftr, I had to allowlist the But it's live now: https://xkbcommon.github.io/libxkbcommon/ |
Upload the doxygen output as artifact from the linux build and use that from the pages job where we combine the static website with our newly build HTML docs. The GitHub actions/download-artefact doesn't work across workflows so we use the other popular one that can do this. The rest of the job is basically copy/paste from the "Static HTML" example GitHub provides.
To make this useful as drop-in replacement, replace the one fixed link to the API docs a relative one.
Refs #326
Build and deployment
select theGithub Actions
option (notDeploy from branch
).