Repository navigation
ci: run the PR test suite on pull requests into develop - #97
Merged
Merged
Conversation
The workflow only triggered on pull_request into master, but master is a release-only branch — every feature PR targets develop. So feature PRs reported no checks at all (`gh pr checks` says "no checks reported") and the suite first ran when a release PR reached master, long after the work had been reviewed and merged into develop. Add develop to the trigger list so analyze, the app tests and both Rust crates' tests gate the branch where work actually lands.
Two follow-ups on the develop trigger: - actions/checkout@v4 runs on Node 20, which the runners now force onto Node 24 with a deprecation annotation. v7 is current (v5 moved to Node 24, v6 writes credentials to a separate file, v7 blocks fork checkouts for pull_request_target / workflow_run and moves to ESM) — none of which affects a plain checkout on a pull_request event. - Add a concurrency group keyed on the PR so pushing again cancels the run still in flight instead of compiling the IronRDP and VNC trees twice side by side. A full run is ~9 minutes, most of it Rust.
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
1 participant
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.
Summary
pr-test.ymlonly triggered onpull_requestintomaster, butmasteris a release-only branch — feature work is PR'd intodevelop. The result: feature PRs reported no checks at all, and the suite first ran when a release PR reachedmaster, long after the work had been reviewed and merged.Changes
developto thepull_request.branchestrigger list, soflutter analyze, the app test suite, and both Rust crates' tests gate the branch where work actually lands.Type of change
ci— CI configuration onlyHow was this tested?
This PR is its own test: it targets
develop, so a check run appearing here is the fix working. (Forpull_requestevents GitHub evaluates the workflow from the PR head, so the new trigger applies to this PR itself.)python3 -c "import yaml; yaml.safe_load(...)"confirms the file still parses and resolves to['develop', 'master'].Notes
Existing open PRs into
develop(e.g. #86) will not retroactively get checks until they are updated — their head branch predates this trigger. Pushing to them, or rebasing once this lands, is enough.Worth considering separately: a
concurrencygroup withcancel-in-progressso a series of pushes to one PR does not run the full Rust + Flutter suite several times over.