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

Workflow to build and deploy docs #52

Merged
merged 26 commits into from
Dec 8, 2024
Merged

Workflow to build and deploy docs #52

merged 26 commits into from
Dec 8, 2024

Conversation

ddundo
Copy link
Member

@ddundo ddundo commented Oct 26, 2024

Closes #4.

@ddundo ddundo self-assigned this Oct 26, 2024
@ddundo ddundo temporarily deployed to github-pages-test November 1, 2024 12:57 — with GitHub Actions Inactive
@ddundo ddundo temporarily deployed to github-pages-test November 1, 2024 13:02 — with GitHub Actions Inactive
@ddundo ddundo temporarily deployed to github-pages-test November 1, 2024 13:06 — with GitHub Actions Inactive
@ddundo
Copy link
Member Author

ddundo commented Nov 1, 2024

@jwallwork23 I think I got this very close to working!

But the error now is Error: Error: Failed to create deployment (status: 404) with build version 50571ec1293b7f1cd9f6267ae94d761b39aee006. Request ID 3141:375322:19A3ED0:3259E2E:6724D261 Ensure GitHub Pages has been enabled: https://github.com/mesh-adaptation/mesh-adaptation-docs/settings/pages. Could you please check that? I don't have the permissions to see/change settings

Edit: I think you just need to choose "GitHub Action" in the pulldown menu under "Source" and then it will automatically recognise this workflow.

@ddundo ddundo temporarily deployed to github-pages-test November 1, 2024 13:53 — with GitHub Actions Inactive
@jwallwork23
Copy link
Member

Okay I changed it to go from this branch temporarily.

@ddundo
Copy link
Member Author

ddundo commented Nov 2, 2024

Great, thanks! This seemed to work (e.g. https://mesh-adaptation.github.io/mesh-adaptation-docs/demos/ping_pong.py.html has no pictures so I guess that's from this build). Let me now run all demos and see if it works.

And yeah, I temporarily have

  deploy:
    name: Deploy to GitHub Pages
    needs: build_docs
    if: ${{ github.ref == 'refs/heads/4_build_docs_ci' }}

but will change the if statement to be main branch before this gets merged, so the website gets deployed only if called from the main branch.

@ddundo ddundo mentioned this pull request Nov 2, 2024
@ddundo
Copy link
Member Author

ddundo commented Nov 2, 2024

This looks good now: https://mesh-adaptation.github.io/mesh-adaptation-docs/.

TODO before merging:

  • Enable UM2N. Will use UM2N's container (Create Docker container for UM2N environment #58) when that gets resolved
  • These jekyll builds need to be disabled in settings and only this workflow should be chosen for deployment (@jwallwork23)
  • Remove references to this branch in build_docs.yml
  • Change url to url: ${{ steps.deployment.outputs.page_url }} (uses the url set in repo's settings)

@ddundo
Copy link
Member Author

ddundo commented Nov 8, 2024

Just to repeat what I said in the meeting: https://mesh-adaptation.github.io/ is the old website (with UM2N) and https://mesh-adaptation.github.io/mesh-adaptation-docs/ is the new one deployed by this workflow (without UM2N)

@jwallwork23
Copy link
Member

This looks good now: https://mesh-adaptation.github.io/mesh-adaptation-docs/.

TODO before merging:

* [ ]   Enable UM2N. Will use UM2N's container ([Create Docker container for UM2N environment #58](https://github.com/mesh-adaptation/mesh-adaptation-docs/issues/58)) when that gets resolved

* [ ]   These jekyll builds need to be disabled in settings and only this workflow should be chosen for deployment (@jwallwork23)

* [ ]   Remove references to this branch in `build_docs.yml`

* [ ]   Change url to `url: ${{ steps.deployment.outputs.page_url }}` (uses the url set in repo's settings)

Congrats @ddundo I've promoted you to "Owner" of the mesh-adaptation org. I trust you to make setting changes without my oversight :)

Would it be possible to progress this PR before setting up the UM2N Docker build or is that a necessary step?

@ddundo
Copy link
Member Author

ddundo commented Nov 17, 2024

Congrats @ddundo I've promoted you to "Owner" of the mesh-adaptation org. I trust you to make setting changes without my oversight :)

That's exciting! Thanks :) but please demote me after this is merged - I trust myself less than you might trust me :)

Would it be possible to progress this PR before setting up the UM2N Docker build or is that a necessary step?

The way it's currently set up, we'd need to import UM2N, right? I temporarily commented that out here. So I thought we'd need a container with UM2N and its dependencies installed. Maybe we could push a (private) docker image as it is in #59 and use it only for this workflow at the moment? I think that would be fine since you now added a conditional import for pytorch3d and it's not important for this PR.

@jwallwork23
Copy link
Member

@jwallwork23 just a summary of what we discussed: I tried to find a way to deploy the build to mesh-adaptation.github.io from this repository, but I couldn't find a way to do it. I could only deploy to https://mesh-adaptation.github.io/mesh-adaptation-docs/. So my suggestions were to either

* Move this workflow to `mesh-adaptation.github.io` repo, or

* have [mesh-adaptation.github.io](https://mesh-adaptation.github.io/) be a static landing page that would link to [mesh-adaptation.github.io/mesh-adaptation-docs](https://mesh-adaptation.github.io/mesh-adaptation-docs/)

This made me think that perhaps we could rename mesh-adaptation-docs repo to just docs? Would look nicer and maybe it's better overall? :)

Renamed to docs. I say we go with the second option. Thanks!

@ddundo ddundo marked this pull request as ready for review December 6, 2024 13:53
@ddundo ddundo requested a review from jwallwork23 December 6, 2024 13:53
@ddundo ddundo added documentation Improvements or additions to documentation PRIORITY We should address this ASAP labels Dec 6, 2024
@ddundo
Copy link
Member Author

ddundo commented Dec 6, 2024

@jwallwork23 this is ready for review now when you have time :)

The CI will trigger every time, run the demos and build docs. So we can check that the docs build properly every time (and it also gives a link to download the html build locally). But it won't deploy it to https://mesh-adaptation.github.io/docs/ unless the Ci was triggered on the main branch (i.e., when merged or when triggered mannually).

Edit: actually I now modified it so that it triggers only when something in docs or in this yml file changes. I think that is sensible. We can still trigger it manually whenever we want to outside of these conditions.

Copy link
Member

@jwallwork23 jwallwork23 left a comment

Choose a reason for hiding this comment

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

Great! Thanks again for setting this up. I have a few questions.

Comment on lines +3 to +14
on:
push:
branches:
- main
paths:
- 'docs/**'
- '.github/workflows/build_docs.yml'
pull_request:
paths:
- 'docs/**'
- '.github/workflows/build_docs.yml'
workflow_dispatch:
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way we could trigger the workflow when there's a push to the main branch of one of Animate/Goalie/Movement/UM2N?

Copy link
Member Author

@ddundo ddundo Dec 8, 2024

Choose a reason for hiding this comment

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

I think this would require calling this workflow from Animate/Goalie/Movement/UM2N, similarly to how we have the "reusable" test workflow here in docs that is called by them. I think this can become a pain to maintain (e.g. how now you had to open 4 separate PRs to fix the docs renaming) so I think it's easier to just trigger this workflow manually when we want to update? Since it's just a press of a button :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Although... now that I think about it. Maybe it would be useful to have an option to call it from branches other than main, so that we can easily check in PRs that the formatting etc. in docs looks good before merging them.

But I guess this would then be a smaller workflow that would only run the demos from that particular repo and not from all 4. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I think this would require calling this workflow from Animate/Goalie/Movement/UM2N, similarly to how we have the "reusable" test workflow here in docs that is called by them. I think this can become a pain to maintain (e.g. how now you had to open 4 separate PRs to fix the docs renaming) so I think it's easier to just trigger this workflow manually when we want to update? Since it's just a press of a button :)

I get that, although in general it's nice if we don't have to remember to do things.

Although... now that I think about it. Maybe it would be useful to have an option to call it from branches other than main, so that we can easily check in PRs that the formatting etc. in docs looks good before merging them.

But I guess this would then be a smaller workflow that would only run the demos from that particular repo and not from all 4. What do you think?

It would be really nice if each repo had a build_docs job that could be triggered to build its part of the docs, but I think it's beyond the scope of this PR. It'd be better to just get this merged first. My original question was just in case you were aware of an easy way to set it up.

Copy link
Member Author

Choose a reason for hiding this comment

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

I get that, although in general it's nice if we don't have to remember to do things.

Sadly I couldn't find a way for the docs repo to "listen" for changes in other repos. But we could set up a workflow with a schedule trigger that would check, let's say, every 12 hours if there have been changes in these repos. And if so, then the docs could be rebuilt?

It would be really nice if each repo had a build_docs job that could be triggered to build its part of the docs, but I think it's beyond the scope of this PR. It'd be better to just get this merged first. My original question was just in case you were aware of an easy way to set it up.

Agreed :) I opened #78

Copy link
Member

Choose a reason for hiding this comment

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

Sadly I couldn't find a way for the docs repo to "listen" for changes in other repos. But we could set up a workflow with a schedule trigger that would check, let's say, every 12 hours if there have been changes in these repos. And if so, then the docs could be rebuilt?

Yeah that could work. A nightly check would probably be sufficient.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good. Opened #79

.github/workflows/build_docs.yml Outdated Show resolved Hide resolved
.github/workflows/build_docs.yml Outdated Show resolved Hide resolved
Copy link
Member

@jwallwork23 jwallwork23 left a comment

Choose a reason for hiding this comment

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

Great stuff, thanks again @ddundo!

@ddundo ddundo merged commit 772bef8 into main Dec 8, 2024
2 checks passed
@ddundo ddundo deleted the 4_build_docs_ci branch December 8, 2024 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation PRIORITY We should address this ASAP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build docs as part of CI
2 participants