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.

Sign up for GitHub

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

chore: Simplify publish_docs workflow #749

Merged
merged 3 commits into from
Aug 30, 2024
Merged

chore: Simplify publish_docs workflow #749

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dc98ffc
chore: Simplify publish_docs workflow
jrconlin Aug 26, 2024
04dc3d4
Merge branch 'master' into docs/747-simplify_docs
jrconlin Aug 29, 2024
ac68982
Merge branch 'master' into docs/747-simplify_docs
jrconlin Aug 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 20 additions & 41 deletions .github/workflows/publish_docs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,57 +1,37 @@
name: Publish docs to pages
run-name: ${{ github.actor }} is publishing docs
on:
# on a successful push
workflow_dispatch: # Allow manual triggering
push:
# only run on pushes to these branches
branches:
- 'master'
- 'main'
- 'docs'
# or run if pushes to these files.
# paths:
# - 'docs/**.md'
check_run:
Copy link
Member Author

Choose a reason for hiding this comment

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

See comment

types:
# Only run if we have a successfully completed branch
- completed
# if desired, we can also use cron like rules to trigger these
# schedule:
# - cron: 0 0 * * *
# Allow for manual triggering
workflow_dispatch:
branches: [master,main]
# paths: ['docs/**.md'] # API docs need building always

# Sets permissions of the GITHUB_TOKEN to allow deployment to GitHub Pages
permissions:
contents: read
pages: write
id-token: write

# Allow only one concurrent deployment. Cancel any in-progress.
concurrency:
group: "pages"
cancel-in-progress: true
group: github-pages
cancel-in-progress: false # Skip any intermediate builds but finish deploying
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a question: do we for sure want a successful deploy should something in docs build fail? I wonder if this would result in us missing failed doc builds. However, it's not critical to the service health so I get the rationale here.

Copy link
Contributor

Choose a reason for hiding this comment

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

If this is related to the concurrency lines directly, then they don't change anything about success of the builds. They only manage how many docs workflow runs can queue in succession, always skipping anything between the first and the last run for this concurrency group. It only changes one thing: don't cancel the one already running and wait for it to finish before starting the most recent.
(the rationale is: you never know if the newer build will be {more} successful, so chances are if the running job can deploy, let it deploy on success… in case the newer run won't build etc.)

This won't change any failed builds behaviour at all. These still fail loudly, and notify accordingly #747 (comment) — and won't deploy if the build fails.


jobs:
build:
runs-on: ubuntu-latest
env:
MDBOOK_ENV: 0.4.24
MERMAID_ENV: 0.12.6
MDBOOK_ENV: 0.4.40
MERMAID_ENV: 0.13.0
DEST_DIR: /home/runner/.cargo/bin
steps:
# these are other job descriptions to call.
- uses: actions/checkout@v3
- name: Configure rust # TODO: can we export building rust and installing mdbook as an artifact and reuse it?
run: |
curl --proto '=https' --tlsv1.2 https://sh.rustup.rs -sSf -y | sh
Copy link
Member Author

Choose a reason for hiding this comment

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

See comment
cargo is now a part of the distribution core and we no longer need to import it directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we still possibly run rustup update? There may be a variance between the distro version and what's currently available.

Copy link
Contributor

@janbrasna janbrasna Aug 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

it's kept uptodate in the runners #747 (comment) ~ and trying to update it manually only triggers warnings #747 (comment) because rustup isn't managing parts of the toolchain: #step:3:16

(similar to installing cargo audit elsewhere janbrasna#step:3:10 … as long as it's in the env setup, it's unnecessarily noisy in the logs then trying to install it manually, IMO better to skip and rely on the env for sanity;)…)

rustup update
export PATH=$PATH:$DEST_DIR
- uses: actions/checkout@v4
- uses: actions/cache@v4
with:
path: |
~/.cargo/bin/
~/.cargo/registry/
~/.cargo/git/
target/
key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
- name: Install mdBook
run: |
export PATH=$PATH:$DEST_DIR
curl -sSL "https://github.com/rust-lang/mdBook/releases/download/v$MDBOOK_ENV/mdbook-v$MDBOOK_ENV-x86_64-unknown-linux-gnu.tar.gz" | tar -xz --directory $DEST_DIR
curl -sSL "https://github.com/badboy/mdBook-mermaid/releases/download/v$MERMAID_ENV/mdbook-mermaid-v$MERMAID_ENV-x86_64-unknown-linux-gnu.tar.gz" | tar -xz --directory $DEST_DIR
# actually build the book
- name: Build the main book
run: cd docs && mdbook build
- name: Build API docs
Expand All @@ -60,11 +40,10 @@ jobs:
- name: Copy cargo docs to API dir
run: mkdir -p docs/output/api && cp -r target/doc/* docs/output/api
- name: Upload artifact
uses: actions/upload-pages-artifact@v1
uses: actions/upload-pages-artifact@v3
with:
path: ./docs/output

# write the pages
deploy:
needs: build
permissions:
Expand All @@ -75,6 +54,6 @@ jobs:
url: ${{ steps.deployment.outputs.page_url }}
runs-on: ubuntu-latest
steps:
- name: Deploy to Github Pages
- name: Deploy to GitHub Pages
id: depolyment
uses: actions/deploy-pages@v2
uses: actions/deploy-pages@v4