Repository navigation
fix order of build - #117
Merged
Merged
Conversation
Benchmark for 30630c0Click to view benchmark
|
Member
|
Let's also do doc generation, mdbook builds and link check in the test.yml workflow then? Everything except the page upload step. |
Benchmark for 00f7132Click to view benchmark
|
Benchmark for 4d122c6Click to view benchmark
|
Collaborator
Author
|
Think I got it working now!
|
Benchmark for 905c8f1Click to view benchmark
|
Benchmark for 1b7e1bdClick to view benchmark
|
Benchmark for 8d536b2Click to view benchmark
|
emuell
reviewed
Sep 29, 2026
| on: | ||
| push: | ||
| branches: [ "master" ] | ||
| # allow running the workflow from the actions tab |
Member
There was a problem hiding this comment.
This was nice to have. I think we should keep it.
| run: cargo test --verbose | ||
|
|
||
| benchmark: | ||
| needs: build |
Member
There was a problem hiding this comment.
I think we need a if: github.event_name == 'pull_request' here too, so benchmark isn't run again when a PR gets merged.
| @@ -1,8 +1,11 @@ | |||
| name: Benchmark | |||
Member
There was a problem hiding this comment.
This got quite complicated now. I think we should merge benchmark.yml into build.yml, so we only have two self-contained workflows:
- build.yml: build, clippy and tests on
pushandpull_request, and the benchmark onlyif:github.event_name == 'pull_request'`. - pages.yml: API docs generation, mdbook and link check on
pushandpull_request, and publishing onlyif: github.ref_name == 'master'.
Both can then have separate concurrency and cache. Actually I don't think we need to specify a cache key then as it will pick up by default the branch name.
Having workflow_dispatch in both also wouldn't hurt.
| |lua, (ud, value): (LuaAnyUserData, LuaValue)| { | ||
| let mut this = ud.borrow_mut::<Self>()?; | ||
| let steps = transpose_steps_array_from_value(lua, value, this.notes.len())?; | ||
| for (note, step) in this.notes.iter_mut().zip(steps.into_iter()) { |
Member
There was a problem hiding this comment.
Let me split those from this PR and fix this in master, so we get a clean baseline...
…ild ci, use mdbook action
emuell
force-pushed
the
fix-build-order
branch
from
September 29, 2026 20:22
58ce421 to
5446bd9
Compare
Benchmark for 147212cClick to view benchmark
|
Member
Something like that could work (untested): name: Build & Test
on:
push:
branches: ["master"]
pull_request:
branches: ["master"]
# allow running the workflow from the actions tab
workflow_dispatch:
# Cancel outdated PR runs
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true
env:
CARGO_TERM_COLOR: always
jobs:
build:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v6
with:
submodules: recursive
- name: Install Rust
uses: actions-rust-lang/setup-rust-toolchain@v2
with:
components: clippy
- name: Build
run: cargo build --verbose
- name: Clippy
run: cargo clippy --verbose -- -D warnings
- name: Tests
run: cargo test --verbose
benchmark:
# Only benchmark PRs against their base branch
needs: build
if: github.event_name == 'pull_request'
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v6
with:
submodules: recursive
- name: Install Rust
uses: actions-rust-lang/setup-rust-toolchain@v2
- name: Benchmark
uses: boa-dev/criterion-compare-action@v3
with:
branchName: ${{ github.base_ref }}pages.yml, deploy would then also run with: on:
push:
branches: ["master"]
pull_request:
branches: ["master"]
# allow running the workflow from the actions tab
workflow_dispatch:
[...]
deploy:
# Deploy the artifact uploaded by the docs job
needs: build
if: github.ref_name == 'master' |
Benchmark for c9bf58bClick to view benchmark
|
Collaborator
Author
|
Made the changes you've requested. How about this last commit: the benchmark will only run if any rust file had been changed inside |
… 'types' to filters
Member
|
Yes, that's nice. Added |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sorry, I overlooked the order being wrong in the build workflow, this should fix it..
Not sure how this could work but it would be nice to add a test around the doc workflow (similar to the benchmark/build test) that would let us know in a PR if the docs fail to build with the PR to avoid such errors being merged.