Skip to content

Remove vendored ConfigKeyKit subrepo; depend on it remotely#417

Merged
leogdion merged 1 commit into
v1.0.0-beta.3from
chore/remove-configkeykit-subrepo
Jun 30, 2026
Merged

Remove vendored ConfigKeyKit subrepo; depend on it remotely#417
leogdion merged 1 commit into
v1.0.0-beta.3from
chore/remove-configkeykit-subrepo

Conversation

@leogdion

@leogdion leogdion commented Jun 30, 2026

Copy link
Copy Markdown
Member

Tracks brightdigit/ConfigKeyKit#6.

Removes the Packages/ConfigKeyKit git-subrepo (vendored copy of brightdigit/ConfigKeyKit) and switches the example backends to the remote package, pinned to the released tag:

.package(name: "ConfigKeyKit", path: "../../Packages/ConfigKeyKit")
→ .package(url: "https://github.com/brightdigit/ConfigKeyKit.git", from: "1.0.0-beta.2")

Changes

  • Examples/BushelCloud, Examples/CelestraCloud, Examples/MistDemo manifests + Package.resolved → ConfigKeyKit pinned at 1.0.0-beta.2 (6949abb).
  • Packages/ConfigKeyKit removed; Packages/ is now empty (MistKit core never depended on it).
  • CI cleanup: removed the now-dead setup-configkeykit action steps and CONFIGKEYKIT_BRANCH env from the BushelCloud and CelestraCloud workflows (BushelCloud.yml, codeql.yml, bushel-cloud-build.yml, the cloudkit-sync composite action incl. its configkeykit-branch input; CelestraCloud.yml, codeql.yml, update-feeds.yml). SwiftPM fetches ConfigKeyKit directly now, so the fresh-build fallback is unnecessary.

Verification

swift package resolve succeeds for all three examples (ConfigKeyKit → 1.0.0-beta.2); the mistdemo executable builds; all seven edited workflow/action YAML files parse. The pre-existing MistDemoApp SwiftUI build errors are unrelated.

Follow-ups

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 64f1fa89-0857-4f60-957b-af40a7b3b394

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/remove-configkeykit-subrepo

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@claude

claude Bot commented Jun 30, 2026

Copy link
Copy Markdown

Code Review

Summary: Removes the vendored Packages/ConfigKeyKit git-subrepo and replaces it with a remote package dependency in all three example backends. Clean housekeeping PR.


Strengths

  • Removes ~55 files / 3207 lines of vendored code that now lives upstream.
  • All three example Package.swift files and their corresponding Package.resolved files are updated consistently.
  • The Package.resolved files pin to a specific commit (6949abb), which provides deterministic builds despite the branch: "main" package declaration.

Concern: branch: "main" vs a tagged version

.package(url: "https://github.com/brightdigit/ConfigKeyKit.git", branch: "main")

branch: "main" means swift package update will silently pull new commits to main. If ConfigKeyKit ships a breaking change, any developer running swift package update (or a clean CI build after the resolved file is invalidated) picks it up without a version bump signal. The existing CI uses CONFIGKEYKIT_BRANCH=main, which explains the choice, but since the goal is to drop the subrepo in favor of a proper dependency, pinning to a released tag (e.g. from: "1.0.0-beta.2") would make this a first-class versioned dependency rather than a floating branch reference. The PR already flags this — worth resolving before promotion from draft.

Follow-ups called out in the PR description

  • CI cleanup: setup-configkeykit action step and CONFIGKEYKIT_BRANCH env var are now vestigial. These should be tracked (or added to this PR if they're straightforward removes) to avoid confusing future CI authors.
  • Subrepo push: BushelCloud/CelestraCloud manifest changes need git subrepo push to land upstream post-approval. This is a process reminder, not a code issue.

Overall: Correct and well-described. The only substantive question is the branch: "main" pin — tag-based versioning would make this a more robust dependency. Everything else is clean. LGTM pending that decision.

@claude

claude Bot commented Jun 30, 2026

Copy link
Copy Markdown

Code Review

This PR cleanly removes the vendored ConfigKeyKit subrepo and wires up all three example backends to the remote package — a clear structural improvement. The manifest changes themselves are correct (dropping the explicit name: label is fine; Swift PM derives the identity configkeykit from the URL, and all .product(name: "ConfigKeyKit", package: "ConfigKeyKit") references resolve correctly). The three Package.resolved files are consistently pinned to the same revision (6949abb). However there are a few issues worth addressing:


🔴 Vestigial setup-configkeykit action will delete Package.resolved on every example CI run

Files: Examples/BushelCloud/.github/workflows/BushelCloud.yml:94,188,258, Examples/CelestraCloud/.github/workflows/CelestraCloud.yml:94,145,191,259, Examples/BushelCloud/.github/actions/cloudkit-sync/action.yml:164, Examples/BushelCloud/.github/workflows/bushel-cloud-build.yml, Examples/CelestraCloud/.github/workflows/update-feeds.yml, codeql workflows

The workflows still call brightdigit/ConfigKeyKit/.github/actions/setup-configkeykit@main (the remote equivalent of the now-deleted local copy). Because CONFIGKEYKIT_BRANCH: main is non-empty, the action's inputs.branch != '' guard is true and the step runs. The sed that replaces the old path form is now a no-op (that string no longer exists in Package.swift), but rm -f Package.resolved still executes unconditionally within that same shell block. This means:

  • Every CI job for BushelCloud and CelestraCloud deletes the lockfile before building.
  • Swift PM then resolves from scratch against branch: "main" HEAD — not against the 6949abb revision committed in Package.resolved.
  • A breaking ConfigKeyKit commit lands silently in CI without any diff in this repo.

Fix: Remove the setup-configkeykit step and the CONFIGKEYKIT_BRANCH env var from all seven affected workflow/action files. The Package.swift files already declare the correct remote URL; no rewriting is needed at build time.


🟡 CONFIGKEYKIT_BRANCH is now a dead env var

Files: Examples/BushelCloud/.github/workflows/BushelCloud.yml:25, Examples/CelestraCloud/.github/workflows/CelestraCloud.yml:25

The variable feeds only the setup-configkeykit step (whose sed pattern no longer matches), so setting it to anything other than main has no effect. A developer trying to pin CI to a specific ConfigKeyKit branch will be silently ignored. This is superseded by the fix above but worth calling out as a separate cleanup.


🟡 branch: "main" is the only floating dependency across all three manifests

Files: Examples/BushelCloud/Package.swift:95, Examples/CelestraCloud/Package.swift:94, Examples/MistDemo/Package.swift:109

Every other external dependency in these manifests uses from: "X.Y.Z" (e.g. BushelKit, IPSWDownloads, swift-configuration) or a named stable-branch pseudo-tag (branch: "v0.0.3" for CelestraKit). branch: "main" is the sole genuinely-moving ref. The committed Package.resolved gives reproducibility today, but any swift package update — including on a fresh clone without a lockfile (see finding above) — pulls whatever main HEAD currently is. The PR description already surfaces the fix: replace with from: "X.Y.Z" using the tag that corresponds to 6949abb, or exact: "6949abb4b7f3e50f7f81d668e6c11aa4721e97fd" if no tag exists yet.


ℹ️ MistKit's examples.yml CI won't trigger on this PR

File: .github/workflows/examples.yml

The workflow is gated on pull_request: branches: [main]; this PR targets v1.0.0-beta.3. The three example packages are therefore not automatically built/validated by MistKit CI for this PR — manual verification (or rebasing/re-targeting to main once ready) is needed before merge.


Summary: The manifest edits are correct; the main action item is removing the seven vestigial setup-configkeykit invocations (and CONFIGKEYKIT_BRANCH) from the example workflows, since the rm -f Package.resolved inside them actively undermines the reproducibility the committed lockfiles are supposed to provide. Pinning to a version tag rather than branch: "main" is a lower-urgency follow-up but aligns with the rest of the dependency strategy.

@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.98%. Comparing base (9894d48) to head (4e057c4).

Additional details and impacted files
@@                Coverage Diff                @@
##           v1.0.0-beta.3     #417      +/-   ##
=================================================
+ Coverage          71.82%   73.98%   +2.15%     
=================================================
  Files                168      156      -12     
  Lines               3844     3698     -146     
=================================================
- Hits                2761     2736      -25     
+ Misses              1083      962     -121     
Flag Coverage Δ
mistdemo-spm-macos 11.19% <ø> (-0.23%) ⬇️
mistdemo-swift-6.2-jammy 11.19% <ø> (-0.23%) ⬇️
mistdemo-swift-6.2-noble 11.44% <ø> (+0.01%) ⬆️
mistdemo-swift-6.3-jammy 11.19% <ø> (-0.23%) ⬇️
mistdemo-swift-6.3-noble 11.19% <ø> (-0.23%) ⬇️
spm 71.55% <ø> (+0.02%) ⬆️
swift-6.1-jammy 71.73% <ø> (ø)
swift-6.1-noble 71.73% <ø> (-0.14%) ⬇️
swift-6.2-jammy 71.59% <ø> (-0.06%) ⬇️
swift-6.2-noble 71.62% <ø> (+0.02%) ⬆️
swift-6.3-jammy 71.62% <ø> (+0.08%) ⬆️
swift-6.3-noble 71.51% <ø> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Removes the `Packages/ConfigKeyKit` git-subrepo (vendored copy of
brightdigit/ConfigKeyKit) and switches the example backends to the remote
package, pinned to the released tag:

  .package(name: "ConfigKeyKit", path: "../../Packages/ConfigKeyKit")
  → .package(url: "https://github.com/brightdigit/ConfigKeyKit.git", from: "1.0.0-beta.2")

- Examples/BushelCloud, Examples/CelestraCloud, Examples/MistDemo manifests +
  Package.resolved updated (ConfigKeyKit pinned at 1.0.0-beta.2 / 6949abb).
- MistKit core never depended on ConfigKeyKit; Packages/ is now empty and removed.
- Removed the now-dead `setup-configkeykit` action steps and `CONFIGKEYKIT_BRANCH`
  env from the BushelCloud and CelestraCloud workflows (BushelCloud.yml, codeql.yml,
  bushel-cloud-build.yml, cloudkit-sync composite action incl. its configkeykit-branch
  input; CelestraCloud.yml, codeql.yml, update-feeds.yml). SwiftPM now fetches
  ConfigKeyKit directly, so the fresh-build fallback action is unnecessary.

Verified: `swift package resolve` succeeds for all three examples (ConfigKeyKit
resolves to 1.0.0-beta.2) and the `mistdemo` executable builds. All seven edited
workflow/action YAML files still parse. The pre-existing MistDemoApp SwiftUI build
errors are unrelated to this change.

Tracks brightdigit/ConfigKeyKit#6.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leogdion leogdion force-pushed the chore/remove-configkeykit-subrepo branch from c2d41d3 to 4e057c4 Compare June 30, 2026 17:34
@leogdion leogdion marked this pull request as ready for review June 30, 2026 17:44
@claude

claude Bot commented Jun 30, 2026

Copy link
Copy Markdown

Code Review — PR #417: Remove vendored ConfigKeyKit subrepo; depend on it remotely

This is a clean, well-scoped dependency-management migration. The intent is clear, the changes are internally consistent, and the PR description is thorough. A few notes:


✅ What looks good

Consistent migration across all three examples. All three Package.swift files receive the same treatment — path dependency → from: "1.0.0-beta.2" remote URL — and all three Package.resolved files are updated with identical pin metadata (revision: 6949abb, version: 1.0.0-beta.2). No example was missed.

Commit hash pinning in Package.resolved. Pinning to a specific commit SHA (6949abb4b7f3e50f7f81d668e6c11aa4721e97fd) in the resolved files means CI gets deterministic builds and is protected against tag mutation / supply chain drift. Good practice.

CI cleanup is thorough. All seven YAML files that referenced setup-configkeykit or CONFIGKEYKIT_BRANCH are updated. The composite action's configkeykit-branch input and all caller sites are gone. No dead references remain.

Removal of the subrepo is clean. The Packages/ tree is fully deleted along with the .gitrepo file and all its vendored CI workflows, lint configs, and scripts — nothing is left dangling.


🔍 Minor observations

CelestraCloud.yml — blank line elided at line ~135. When the Setup ConfigKeyKit step is removed between the MistKit setup step and - uses: brightdigit/swift-build@v1, the step separator blank line that was there for readability disappears. The YAML is still valid, but the other removal hunks in the same file keep a blank line between adjacent steps. Worth adding one for consistency, though this is purely cosmetic.

from: "1.0.0-beta.2" permits future compatible versions on swift package update. Because Package.resolved is committed, CI always uses the pinned revision and this has no practical impact. But if a developer runs swift package update locally, SwiftPM could resolve to 1.0.0-beta.3 or 1.0.0 without warning. This is the conventional SwiftPM pattern for example apps and is fine; just worth knowing. If strict control over when ConfigKeyKit upgrades is ever desired, .exact("1.0.0-beta.2") would make the intent explicit — but I wouldn't change it for examples.

bushel-cloud-build.yml hardcoded branch: main in the removed step. The removed Setup ConfigKeyKit step in this workflow used branch: main directly rather than the CONFIGKEYKIT_BRANCH env var. This was a pre-existing inconsistency that is now correctly gone; worth calling out only because it confirms the hunk is safe to delete (it was already diverged from the env-var-driven pattern).


💡 Process note

The PR description already captures the key follow-up: BushelCloud and CelestraCloud are subrepos, so these changes need git subrepo push to land upstream after merge. That's handled out-of-band and well-documented. No action needed here.


Overall: LGTM. The migration is correct, complete, and safe. The blank-line cosmetic nit in CelestraCloud.yml is the only thing I'd consider touching before merge.

@leogdion leogdion merged commit a61b0a0 into v1.0.0-beta.3 Jun 30, 2026
71 of 73 checks passed
@leogdion leogdion deleted the chore/remove-configkeykit-subrepo branch July 1, 2026 00:29
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.

1 participant