Skip to content

V1.0.0 beta.3#408

Draft
leogdion wants to merge 3 commits into
mainfrom
v1.0.0-beta.3
Draft

V1.0.0 beta.3#408
leogdion wants to merge 3 commits into
mainfrom
v1.0.0-beta.3

Conversation

@leogdion
Copy link
Copy Markdown
Member

@leogdion leogdion commented Jun 4, 2026

No description provided.

leogdion and others added 3 commits June 1, 2026 14:57
Restores the full docs/ tree removed from main in the previous commit,
sourced from docs/talk-prep-archive so the authentication-middleware path
fix (aa9e36d) is folded in. Because this add lands after the deletion point,
merging v1.0.0-beta.3 into main will re-add docs/ without conflict.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 4, 2026

Important

Review skipped

Draft detected.

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: aa4ae107-a993-4a47-8967-e20c39fad9a7

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 v1.0.0-beta.3

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 and usage tips.

@claude
Copy link
Copy Markdown

claude Bot commented Jun 4, 2026

Code Review: V1.0.0 beta.3

Overview

This PR advances CelestraCloud and BushelCloud examples to the beta.3 API, with the centerpiece change being a refactoring of CelestraCloud's configuration system to use ConfigKeyKit. It also includes CI workflow updates, copyright year corrections, license header additions, and new documentation articles.


CI / Workflow Changes

Removing scheme: inputs from swift-build — The removal of the hardcoded scheme: parameters in .github/workflows/MistKit.yml and MistDemo.yml is clean and likely reflects the action having learned to auto-detect the scheme.

Branch tag format change — The MISTKIT_BRANCH env var changes from v1.0.0-beta.2 to 1.0.0-beta.2 (dropping the v prefix). Make sure the setup-mistkit action resolves both forms — if it expects a v-prefixed tag this will silently fall back to a branch named 1.0.0-beta.2 instead of the release tag.

CONFIGKEYKIT_BRANCH: main — Pinning to main rather than a release tag is fine for now but worth revisiting before a stable release to ensure reproducibility.


CelestraCloud: ConfigKeyKit Refactor

What changed

  • ConfigurationKeys now stores typed ConfigKey<T> / OptionalConfigKey<T> values (from ConfigKeyKit) instead of paired raw strings.
  • ConfigurationLoader gains overloaded read(_:) helpers that iterate ConfigKeySource.allCases (CLI to ENV) and apply defaults automatically, eliminating the previous triple-expression "try dot-notation key / try env-key / use default" pattern.
  • ConfigSource, ConfigurationError, and the deprecated CelestraConfig.createCloudKitService() are removed.

Positives

  • The duplicated key-pair pattern (e.g. containerID + containerIDEnv) is gone; the lookup cascade is now centralized.
  • internal import Foundation visibility fix on CelestraConfig.swift follows the project's explicit-ACL-on-imports convention.
  • Removing the deprecated createCloudKitService() reduces dead-code surface.

Issues / Suggestions

1. The read overloads may be reinventing ConfigKeyKit.
The six private func read() overloads each walk ConfigKeySource.allCases manually. If ConfigKeyKit's ConfigKey/OptionalConfigKey types already expose a value(from: ConfigReader) or similar method, this layer is redundant. Worth checking whether a direct call exists — if so, the entire MARK section can collapse to a single generic.

2. read(_ key: ConfigKey<Bool>) CLI semantics are surprising.
The boolean helper treats any present CLI key as true regardless of its string value. This is correct for flag-style arguments (--skip-robots-check) but could cause subtle breakage if a caller passes --skip-robots-check false — the value "false" would still return true. The docstring covers this, but a comment in ConfigurationKeys that boolean keys are flags (not value options) would prevent future confusion.

3. readDate implementation is worth verifying.

private func readDate(forKey key: String) -> Date? {
  configReader.string(forKey: ConfigKey(key), as: Date.self)
}

This relies on configReader.string(forKey:as:) accepting Date.self to perform ISO8601 conversion. That's a non-standard overload — confirm this API exists on Configuration.ConfigReader and works as expected. If it doesn't, this will silently return nil for all date keys.

4. Missing read(_ key: OptionalConfigKey<Double>) overload.
There's no read(_ key: OptionalConfigKey<Double>) helper. A future Double? config key would produce a compile error. Not a blocker, but worth adding for symmetry with the other optional overloads.

5. readString(forKey:) and readInt(forKey:) helpers are now only used internally.
These remain post-refactor as primitives for the new read() overloads — that's fine architecturally. Just verify Periphery doesn't flag them as dead code.


BushelCloud: Test File Housekeeping

Copyright year updates (2025 to 2026) and missing MIT license headers added across test files. The lint.sh change to run header.sh over Tests/ (in addition to Sources/) explains this — good catch.

FieldValueURLTests.swift was previously attributed to Claude Code; now correctly attributed to Leo Dion.


Documentation

New articles under docs/cloudkit-guide/ (auth, deployment, MistKit rebuild series) plus docs/why-mistkit.md and supporting talk/social assets. No code concerns.


Summary

The ConfigKeyKit refactor is a clear improvement — the duplicated key-pair pattern is gone and the cascade logic is centralized. Three things to verify before merging:

  1. Confirm the v-prefix change doesn't break setup-mistkit action tag resolution.
  2. Confirm configReader.string(forKey:as: Date.self) is a real Configuration API (or replace with explicit ISO8601 parsing).
  3. Check ConfigKeyKit for a native value(from:) method to avoid manually walking ConfigKeySource.allCases in six separate overloads.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.82%. Comparing base (7fe8090) to head (9894d48).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #408      +/-   ##
==========================================
+ Coverage   71.67%   71.82%   +0.15%     
==========================================
  Files         168      168              
  Lines        3844     3844              
==========================================
+ Hits         2755     2761       +6     
+ Misses       1089     1083       -6     
Flag Coverage Δ
mistdemo-spm-macos 11.42% <ø> (ø)
mistdemo-swift-6.2-jammy 11.42% <ø> (ø)
mistdemo-swift-6.2-noble 11.42% <ø> (ø)
mistdemo-swift-6.3-jammy 11.42% <ø> (ø)
mistdemo-swift-6.3-noble 11.42% <ø> (ø)
spm 71.52% <ø> (-0.09%) ⬇️
swift-6.1-jammy 71.73% <ø> (+0.16%) ⬆️
swift-6.1-noble 71.86% <ø> (-0.03%) ⬇️
swift-6.2-jammy 71.65% <ø> (+0.08%) ⬆️
swift-6.2-noble 71.59% <ø> (-0.03%) ⬇️
swift-6.3-jammy 71.54% <ø> (-0.09%) ⬇️
swift-6.3-noble 71.59% <ø> (+0.05%) ⬆️

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.

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