| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
@hannesa2 one call worth your eye before this leaves draft: the TODO beside the "Run tests" step. Left as it stands, CI keeps running GitXTests twice on builds that carry the signing secrets: once in "Run unit tests", and again inside "Run tests", since the scheme tests every target and that step passes no -only-testing. This PR preserves that on purpose, so that it changes only how the workflow says things and not what it does. Swapping make all-tests for make ui-test narrows the second run to GitXUITests, which is all the screenshots need, and drops the repeat. That is a one-word edit if you want it, either here or later. |
Sorry, something went wrong.
There was a problem hiding this comment.
Updates the PR build workflow to invoke the existing Makefile targets for dependency setup, testing, archiving, and packaging so CI and local developer builds run the same commands and avoid workflow/Makefile drift.
Changes:
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Sorry, something went wrong.
Call the targets that already define these commands, so that the workflow and a developer's machine run the same thing rather than two copies of it. - Replace the dependency, test, archive and packaging commands with the targets that hold them - Keep the wider test step as it stands, repeated unit tests and all, with a note about narrowing it later - Leave the certificate, notarization and upload steps alone, being the runner's business and not runnable locally A command defined once cannot drift from itself, and a mistake in a target now fails the build that depends on it.
| Back | FazBrowse Home | New Git URL |
Call the targets that already define these commands, so that the
workflow and a developer's machine run the same thing rather than two
copies of it that drift apart.
the targets that hold them
the runner's business and not runnable locally
The targets these steps call came from #590, now merged, so this is a
single commit against master.
Where CI attaches to the target graph
The same picture as #590, which explains the targets themselves. What
matters here is which nodes the workflow now enters at.
Before:
After, with the three targets #590 adds shaded:
This PR points five steps at deps, unit-test, all-tests,
archive and package-signed. None of them is an aggregate: CI enters
low in the graph every time, while a person keeps using the targets
above them, pre-build, test and dmg-signed. That is why deps
and package-signed had to be split out of their parents at all. CI
has checked the submodules out through its own step and builds the
archive in another, so a step calling pre-build would redo the
checkout and a step calling dmg-signed would archive a second time
per architecture.
The duplicate test run is kept, deliberately
CI's "Run tests" step passes no -only-testing, and the scheme tests
every target, so it runs GitXTests a second time after the "Run unit
tests" step has already run them. This change keeps that exactly as it
is, calling make all-tests, which is the target written to match it.
make ui-test narrows the run to GitXUITests, which is all the
screenshots need, and would drop the repeat. That is a change to what
CI does rather than to how it says it, so it is left alone here and
marked with a TODO beside the step. Say the word and it is a one-word
edit.
Test plan:
runners
writes the bundle, running the unit suites and GitXScreenshotTests
runs today, with the same paths
not run on a fork PR and are verified by reading and by make -n
only