Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
wmhn1872265132
added a commit
to wmhn1872265132/NVDA
that referenced
this pull request
Jun 25, 2025
…ntroduced in advance
See test results for failed build of commit 41fa929994 |
SaschaCowley
requested changes
Jun 26, 2025
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR enhances CI/CD configurability for forks by externalizing build settings into GitHub variables and updates related documentation and workflows.
- Adds explanatory comment in
setSconsArgs.ps1and retains AppX build flag - Expands
ci/README.mdwith detailed setup steps and variable descriptions - Refactors
.github/workflows/testAndPublish.ymlto consume repository vars, default values, and split symbol jobs
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| ci/scripts/setSconsArgs.ps1 | Added a comment about AppX support and conditional target inclusion |
| ci/README.md | Expanded documentation on build types, environment variables, and setup |
| .github/workflows/testAndPublish.yml | Switched hardcoded defaults to vars, updated feature flags, and separated symbol creation/upload jobs |
Comments suppressed due to low confidence (3)
ci/README.md:59
- Replace the placeholder '*' with a clear reference (e.g., '0') to indicate the default starting build count.
This means our first build will be numbered something like 100,001 not *
.github/workflows/testAndPublish.yml:191
- Add a default fallback (
|| '') forapiSigningTokento avoid workflow failures when the secret isn't defined in a fork.
apiSigningToken: ${{ github.event_name == 'push' && secrets.API_SIGNING_TOKEN }}
.github/workflows/testAndPublish.yml:318
- [nitpick] Consider relocating
mozillaSyms.pyintoci/scriptsand updating the workflow path for improved consistency and maintainability.
# TODO: this script should be moved to ci/scripts
5 tasks
seanbudd
commented
Jun 26, 2025
Co-authored-by: Sascha Cowley <16543535+SaschaCowley@users.noreply.github.com>
SaschaCowley
approved these changes
Jun 26, 2025
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.
Link to issue number:
Part of #17878
Summary of the issue:
Forks of NVDA cannot customise parts of the CI/CD pipeline without committing to code.
This allows forks to maintain a clean diff with
nvaccess/nvda.Right now several environment variables and feature flags are set in
testAndPublish.yml, the CI/CD script.Forks should be able to maintain their CI/CD pipeline without making commits, as commits dirty the fork and make merging upstream harder.
For AppVeyor, developers could create a gist for appveyor.yml, point AppVeyors UX towards it, and host it separately to their repo fork.
This can't be done with GitHub actions.
As such, all variables which need to be customizable by forks should be able to be set from GitHub variable contexts.
such as:
The current GitHub actions design doesn't allow for a separate environment to override
.github/workflows/testAndPublish.yml.Description of user facing changes:
None
Description of developer facing changes:
Forks can now more easily maintain a custom build set up.
Documentation has been added on how to do this.
Forks should work clean out of the gate, with features disabled by default, for everything except tagged releases.
We also have more clear documentation on how to set up the NVDA repository for builds
Description of development approach:
Config is moved out of source control into github variables. sensible defaults are provided for forks
Testing strategy:
Test builds
Known issues with pull request:
Code Review Checklist:
@coderabbitai summary