FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Run the CI build steps through the Makefile targets by goneng · Pull Request #591 · gitx/gitx · GitHub

/ gitx Public

Run the CI build steps through the Makefile targets - #591

Open
goneng wants to merge 1 commit into
gitx:masterfrom
goneng:run-the-ci-steps-through-make
Open

Run the CI build steps through the Makefile targets#591
goneng wants to merge 1 commit into
gitx:masterfrom
goneng:run-the-ci-steps-through-make

Conversation

goneng commented Aug 23, 2026
edited
Loading

Copy link
Copy Markdown
Contributor

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.

  • Replace the dependency, test, archive and packaging commands with
    the targets that hold them
  • Leave the certificate, notarization and upload steps alone, being
    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:

flowchart TD
  pb["pre-build<br>builds deps inline"] --> gss["git-submodule-sync"]
  ds["dmg-signed<br>packages inline"] --> ar["archive"]
  bp["build-project"] --> ar
  app["app"] --> ar
  dmg["dmg"] --> app
  run["run"] --> build["build"]
  ut["unit-test"]
  uit["ui-test"]
Loading

After, with the three targets #590 adds shaded:

flowchart TD
  pb["pre-build"] --> gss["git-submodule-sync"]
  pb --> deps["deps"]
  ds["dmg-signed"] --> ar["archive"]
  ds -. "via make" .-> ps["package-signed"]
  bp["build-project"] --> ar
  app["app"] --> ar
  dmg["dmg"] --> app
  run["run"] --> build["build"]
  ut["unit-test"]
  uit["ui-test"]
  at["all-tests"]
  classDef new fill:#E1F5EE,stroke:#0F6E56,color:#04342C
  class deps,ps,at new
Loading

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:

  • This PR's own CI run is green on both architectures, with make deps, make unit-test ARCH=arm64 and make archive running on the
    runners
  • make all-tests ARCH=arm64 RESULT_BUNDLE=<path> is green locally and
    writes the bundle, running the unit suites and GitXScreenshotTests
  • make -n for every converted step reproduces the command that step
    runs today, with the same paths
  • The screenshot and packaging steps need signing secrets, so they do
    not run on a fork PR and are verified by reading and by make -n
    only

goneng force-pushed the run-the-ci-steps-through-make branch from 7ab4b1c to 4dd22b9 Compare August 23, 2026 23:22
goneng force-pushed the run-the-ci-steps-through-make branch from 4dd22b9 to 916ba4b Compare August 24, 2026 11:50

goneng commented Aug 24, 2026
edited
Loading

Copy link
Copy Markdown
Contributor Author

@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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Pull request overview

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:

  • Replace inline dependency bootstrapping with make deps.
  • Replace inline xcodebuild test invocations with make unit-test and make all-tests (including result bundle wiring).
  • Replace inline archive/export/DMG/ZIP packaging commands with make archive and make package-signed, keeping signing/notarization/upload steps as CI concerns.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

goneng force-pushed the run-the-ci-steps-through-make branch from 916ba4b to 6607cc2 Compare August 24, 2026 13:05
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.
goneng force-pushed the run-the-ci-steps-through-make branch from 6607cc2 to 87a2b34 Compare August 24, 2026 14:45
goneng marked this pull request as ready for review August 24, 2026 14:56
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL