Skip to content

Migrate to Swift 6.4: mise-based lint tooling, rebuilt CI, and source cleanups#127

Merged
leogdion merged 22 commits into
v1.0.0from
brightdigit-com-260406
Jun 21, 2026
Merged

Migrate to Swift 6.4: mise-based lint tooling, rebuilt CI, and source cleanups#127
leogdion merged 22 commits into
v1.0.0from
brightdigit-com-260406

Conversation

@leogdion

@leogdion leogdion commented Jun 19, 2026

Copy link
Copy Markdown
Member

Migrates the package to the Swift 6.4 toolchain, modernizes the lint/format tooling, and rebuilds the CI workflow accordingly.

CI

  • Replace the CI workflow with a Swift 6.4 nightly pipeline (swiftlang/swift:nightly-6.4.x-noble container) covering Linux, macOS, Windows, Android, and the Apple-platform simulator suite (iOS/watchOS/tvOS).
  • Add a configure matrix job that scales coverage by event/branch (small set always; full matrix on main/semver/dispatch/PRs into them; Windows as its own tier).
  • Build Linux in standard, wasm, and wasm-embedded variants (wasm legs build-only), gated on the ENABLE_WASM repo variable.
  • Process coverage via a pinned brightdigit/swift-coverage-action fork (handles the Swift 6.4 swiftbuild output layout) and upload to Codecov; install curl where the uploader needs it.
  • Run linting in a Swift 6.4 container via jdx/mise-action.

Tooling / Lint

  • Adopt mise (.mise.toml) to manage swift-format, SwiftLint, and Periphery, replacing the Mint-based setup.
  • Rewrite Scripts/lint.sh to bootstrap and run tools through mise, accumulate errors instead of exiting early, build tests for compilation checks, and restrict in-place rewrites (header.sh, periphery) to local runs.
  • Tighten .swiftlint.yml: enable missing_docs, one_declaration_per_file, relax/retune length and complexity thresholds, refresh exclusions, and add a custom no_unchecked_sendable rule.
  • Tighten .swift-format: enable NeverForceUnwrap, NeverUseForceTry, NeverUseImplicitlyUnwrappedOptionals, UseLetInEveryBoundCaseVariable, and ValidateDocumentationComments; switch file-scoped privacy to fileprivate.
  • Remove the obsolete .swiftformat and .hound.yml configs.
  • Update Scripts/header.sh to skip generated files and use straight quotes in the license header.

Swift 6.4 / Concurrency

  • Bump the minimum macOS deployment target to 13 in Package.swift.
  • Update case let patterns to per-binding let to satisfy the format rules.
  • Scope the necessary @unchecked Sendable suppression to the legacy JSONDecoder retroactive conformance on Swift < 5.7 only.

Source

  • Extract SiteCollectionDirectoryBuilder out of SiteDirectoryBuilder.swift into its own file.
  • Extract PodcastEpisodeProperties out of PodcastEpisode.swift into its own file.
  • Normalize MIT license headers across all source files to use straight quotes.
  • Annotate the RSS 2.0 webMaster API with a scoped inclusive_language suppression (spec element name; public API cannot be renamed).

🤖 Generated with Claude Code

leogdion and others added 8 commits June 10, 2026 15:17
… ci]

Apply the issue #54 config migration to the vendored BrightDigit libraries
(SyndiKit, TransistorPublishPlugin, Contribute, ContributeWordPress,
NPMPublishPlugin, YoutubePublishPlugin) using the MistKit/SundialKit library
config variant:

- .swiftlint.yml: strict library ruleset + no_unchecked_sendable custom rule
- .swiftformat -> .swift-format: Apple swift-format JSON
- .mise.toml: swift-format 604 (Swift 6.4-aligned), swiftlint, periphery
- Scripts/lint.sh (mise-based, replaces Mint) + Scripts/header.sh
- CI workflow -> brightdigit/swift-build@v1 (ubuntu + macos) + STRICT lint job
- Remove obsolete .hound.yml

Configs + CI only; source reformatting/headers deferred (matches main repo).
SwiftTube and Spinetail skipped (slated for OpenAPI-generator migration).
Committed in the monorepo only; no git subrepo push performed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a `test-packages` matrix job to main.yaml that builds, tests, and lints
the 6 vendored BrightDigit packages in-place. In the monorepo checkout every
sibling already sits at its relative-path location, so each package resolves
with zero dependency reconstruction — replacing the per-package setup-deps CI.

- detect-changes: add `packages` paths-filter + `packages-changed` output
- remove standalone workflows + orphaned setup-deps from the 4 rel-dep
  packages (ContributeWordPress, Transistor, NPM, Youtube); their standalone
  CI was synthetic (relative deps are committed)
- keep SyndiKit + Contribute standalone CIs (genuinely standalone libraries)
- bump all GitHub Actions to latest: checkout v6, cache v5, upload-artifact
  v7, download-artifact v8, dorny/paths-filter v4 (swift-build@v1 unchanged)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Post-rebase cleanup after replaying the config migration onto v1.0.0:
- Fix swiftlint/swift-format violations surfaced by the stricter
  standardized configs (explicit ACLs, formatting, XCTFail messages,
  XCTUnwrap instead of force-unwraps, doc-comment corrections)
- Inline-disable intentional exceptions in source: iTunes* type names,
  RSS-spec webMaster, @unchecked Sendable fallback, large test fixtures
- Split files violating one_declaration_per_file
- Normalize license headers via Scripts/header.sh (straight quotes)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Update requirements to match Package.swift (Swift 5.10 / Xcode 15.3,
  iOS 13 / watchOS 6 / tvOS 13 / macOS 10.15 deployment targets)
- Bump install snippet to from: "1.0.0"
- Fix transposed JSONFeed column in the Feedable mapping table
  (siteURL -> homePageUrl, summary -> description)
- Fix invalid Swift in code snippets (Data(contentsOf:) usage,
  missing paren in case pattern)
- Fix typos and stale LICENSE link

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 19, 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: 1c4a61bb-0986-4511-aa7b-46d14321bd7f

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 brightdigit-com-260406

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

claude Bot commented Jun 19, 2026

Copy link
Copy Markdown

Code Review — PR #127: Integrate brightdigit.com vendored changes + Swift 6.4 CI

Overview

This PR syncs the monorepo-vendored copy of SyndiKit back to the standalone repository and modernizes the CI/toolchain setup to match the BrightDigit Swift 6.4 template. The changes touch 145 files across CI workflows, linting configuration, package requirements, and source/test code.


CI / Workflow (.github/workflows/)

Positives:

  • Clean, minimal new workflow that delegates to the reusable brightdigit/swift-build@v1 action — much simpler than the 380-line workflow it replaces.
  • concurrency cancel-in-progress is preserved.
  • mise cache using .mise.toml hash is the right approach.

Concerns:

  • Significant coverage regression: The old workflow ran Ubuntu (Swift 5.10–6.3, jammy + noble), macOS (three Xcode versions), Windows (three Swift versions × two OS), Android, iOS, watchOS, tvOS, visionOS, and a Swift source compatibility suite. The new workflow runs only Ubuntu 6.3-noble and macOS-15. This is a major reduction in the CI matrix for a library that supports many platforms. Even if this is intentional (monorepo handles the full matrix), the PR description and inline comments don't make this trade-off explicit.
  • Branch trigger leak: The push trigger hardcodes brightdigit-com-260406 (the PR branch itself). This should be removed before merging so future branches don't silently opt out of CI.
  • mise install via curl | sh: Piping curl directly to sh is a supply-chain risk in CI. Consider using the official jdx/mise-action GitHub Action instead, which pins the version.
  • Codecov removed: The new workflow has no coverage upload. If coverage tracking matters, this should be noted explicitly or re-added.

Tooling Migration (Mint → mise)

Positives:

  • .mise.toml is a clean replacement for Mintfile with pinned tool versions.
  • lint.sh is better organized and the STRICT/default modes are clearer.
  • Adding swift build --build-tests to the lint step catches compilation errors that linting alone misses — good addition.
  • Restricting header.sh to local-only (skipped when $CI is set) is correct; running it in CI was always risky.

Concerns:

  • set -e was removed from lint.sh with the comment "allow script to continue running." The run_command wrapper already handles error counting, but removing set -e means unexpected shell errors (e.g., pushd failing) will be silently ignored. Consider reinstating set -e for the non-run_command shell operations.
  • Missing LINT_MODE=NONE exit path: The LINT_MODE=NONE exit now comes after mise install. If someone sets LINT_MODE=NONE to skip all linting, they'll still pay the cost of mise install. Moving the NONE check before TOOL_CMD setup would be more efficient (the INSTALL mode exit is already there for this reason).

SwiftLint / swift-format Configuration Changes

Positives:

  • Enabling missing_docs enforces documentation on public APIs — aligns with the DocC focus.
  • one_declaration_per_file helps enforce the pattern already used by this codebase.
  • NeverForceUnwrap, NeverUseForceTry, NeverUseImplicitlyUnwrappedOptionals all enabled in .swift-format — stricter safety rules.
  • Custom no_unchecked_sendable rule is a good project-specific guard for Swift 6 concurrency correctness.

Concerns:

  • cyclomatic_complexity warning raised from 6 → 12 and file_length error raised from 550 → 300 lines: The first change significantly relaxes complexity limits; the second tightens file length. Verify these are intentional and consistent. If files were being split to comply with 550-line limits, a 300-line error threshold may now flag many files.
  • function_body_length warning threshold removed (- 30 dropped): The warning level was 30 lines and error was 50; now it's 50/76. This relaxes the warning substantially.
  • line_length raised from 90 → 108 (warning) and 90 → 200 (error): A 200-char error limit is extremely permissive for a library. The 108-char warning limit also diverges from the 90 documented in CLAUDE.md. If this PR merges, CLAUDE.md should be updated.
  • type_name exclusions removed: The iTunesDuration, iTunesEpisode, iTunesOwner, iTunesImage exclusions are gone, and the rule now just uses min_length: 3 / max_length: 50. These types start with lowercase i — verify the linter won't now flag them (or that a different rule handles this).
  • Tests excluded from linting removed: The line - Tests was removed from the excluded list, meaning tests are now linted. This is generally good, but it caused a wave of // swiftlint:disable suppressions in the test files, which is noise.

Package.swift

  • macOS(.v10_15)macOS(.v13): This is a breaking change for any consumer currently targeting macOS 10.15–12. It should be explicitly called out in the PR description and the changelog/release notes if this library follows SemVer. There's no comment or rationale in the diff.

Source Code Changes

Positives:

  • Moving SiteCollectionDirectoryBuilder from SiteDirectoryBuilder.swift into its own file correctly applies the one_declaration_per_file rule.
  • License header normalization (smart quotes → escaped quotes) is a correctness fix for the shell script template.
  • Doc comment cleanups (removing redundant - Returns: in initializers) improve doc quality.

Concerns:

  • Stale comment in SiteCollectionDirectoryBuilder.swift: The new file (line ~40) has /// A struct representing an Atom category. as the lead line of the directory(fromCollection:) doc comment. This appears to be a copy-paste artifact from AtomCategory and is incorrect.
  • SiteLanguage.swift doc comment: /// A struct representing an Atom category. also appears at line ~51 of SiteLanguage.swift. Same copy-paste issue.

Test Changes

Positives:

  • Renaming c1/c2 to category1/category2 and t1/t2 to tag1/tag2 improves readability and removes the identifier_name warnings.
  • Replacing URL(strict: "...")! with try #require(URL(strict: "...")) in UTF8EncodedURLTests is the correct Swift Testing idiom and removes the forced unwrap.
  • Extracting long URL strings into hostImageURLString / guestImageURLString constants avoids line-length violations cleanly.
  • guard case let .url(url)guard case .url(let url) pattern update aligns with UseLetInEveryBoundCaseVariable.

Concerns:

  • try! in test fixtures: Content.ResultDictionary.swift still uses try! for xmlFeeds, jsonFeeds, wordpressDataSet, opml, and blogs. The new linting rules enforce NeverUseForceTry, so each site now needs both a // swift-format-ignore: NeverUseForceTry and a // swiftlint:disable:next force_try comment. This is noisy and signals these should be refactored to throws-propagating lazy initializers or setUp methods rather than force-trying in static properties. The suppressions are a pragmatic short-term fix but leave technical debt.
  • RSSCodedTests.swift suppressions: The file now has // swiftlint:disable:this file_name, // swiftlint:disable file_length, and // swiftlint:disable:next type_body_length at the top. This is a sign the file is too large and the long-term fix is splitting it, not suppressing.

Summary

Area Status
CI platform coverage ⚠️ Significantly reduced — needs explicit justification
Branch trigger cleanup ⚠️ PR branch hardcoded in push trigger
Tooling migration (Mint → mise) ✅ Good approach, minor shell-safety concerns
Breaking min deployment target ⚠️ Undocumented breaking change (macOS 10.15 → 13)
Linting rule changes ⚠️ Some thresholds loosened; CLAUDE.md needs update
Copy-paste doc comment errors ❌ Stale "Atom category" comments in two new/moved files
Test improvements ✅ Variable naming, force-unwrap removal
try! suppressions in tests ⚠️ Pragmatic but noisy; consider refactoring

The structural changes (file splits, doc comment cleanup, tooling migration) are well-executed. The main items to address before merge: (1) document or reconsider the CI coverage reduction, (2) remove the PR branch from the push trigger, (3) fix the stale doc comments in SiteCollectionDirectoryBuilder.swift and SiteLanguage.swift, and (4) document the macOS minimum version bump as a breaking change.

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

claude Bot commented Jun 19, 2026

Copy link
Copy Markdown

Code Review: Integrate brightdigit.com vendored changes + Swift 6.4 CI

Summary: This PR syncs the brightdigit.com monorepo-vendored copy of SyndiKit back to the standalone repo and updates CI/tooling to align with the BrightDigit Swift 6.4 template. Most changes are CI infrastructure, tooling migration (Mint to mise), linting config, and mechanical code reformatting. There are a few items that need attention before merge.


Must Fix

1. Feature branch name hardcoded in CI workflow

The new workflow hard-codes the PR own branch name (brightdigit-com-260406) as a push trigger in .github/workflows/SyndiKit.yml line 10. This is a leftover from the monorepo copy and should be removed before merging — otherwise CI will silently stop running on pushes to any future branch once this branch is gone.

2. Major loss of CI coverage

The old syndikit.yml (380 lines) is replaced by SyndiKit.yml (48 lines). The following are no longer tested:

  • Windows (2022, 2025 with Swift 6.1-6.3)
  • Android (API 28, 33, 36 with Swift 6.1-6.3)
  • iOS, watchOS, tvOS, visionOS simulator tests
  • Swift version matrix (5.10, 6.0, 6.1, 6.2 - only 6.3 remains)
  • Ubuntu jammy (only noble remains)
  • Code coverage / Codecov integration
  • Swift source compatibility suite

If the intent is to permanently simplify CI, this is a significant regression in cross-platform assurance for a library that targets Linux, Windows, Android, and Apple platforms. If this is a temporary step for the monorepo sync, that should be documented in the PR description. Consider either restoring the full matrix or tracking the removed jobs as follow-up issues.


Issues

3. Prerelease tooling pinned in .mise.toml

swift-format is pinned to 604.0.0-prerelease-2025-12-17. Prerelease formatting tools can produce different output across snapshots, making diffs noisy. Prefer a stable release when available; if a stable 604.x release is not yet available, add a comment tracking when to bump.

4. Stale copy-paste doc comment in three newly-separated files

In SiteCollectionDirectoryBuilder.swift, PodcastEpisodeProperties.swift, and YouTubeIDProperties.swift, the first doc comment line reads "A struct representing an Atom category." None of these types are Atom categories. Since missing_docs is now enabled and ValidateDocumentationComments is true, these will pass validation while being semantically wrong. Fix the leading summary line in each.

5. set -e removed from lint.sh without equivalent safety

The manual ERRORS counter only wraps explicit run_command calls. Bare commands (pushd, popd, mise install) are not wrapped and will not increment ERRORS on failure. If mise install fails silently, the script proceeds to lint with potentially wrong tool versions. Consider wrapping all commands with run_command or restoring set -e for the mise bootstrap step.

6. swift build --build-tests added to the lint path

lint.sh now calls swift build --build-tests unconditionally, substantially increasing lint time and requiring a working Swift toolchain just to lint. This is not mentioned in the PR description. If the goal is catching compilation errors, CI already does that via the build jobs.


Improvements (Noted Positively)

  • one_declaration_per_file rule enabled: Moving SiteCollectionDirectoryBuilder, PodcastEpisodeProperties, and YouTubeIDProperties to their own files improves navigability.
  • missing_docs enabled: Good practice for a public-facing library. The Throws additions and Returns cleanup throughout are solid.
  • no_unchecked_sendable custom rule: Enforcing proper Sendable conformance is a meaningful safety improvement. The JSONDecoder Swift pre-5.7 carve-out is correctly documented with an inline explanation.
  • webMaster disable comment in RSSChannel.swift: The RSS 2.0 spec name is correctly preserved with a clear justification comment.
  • Pattern binding style update from "case let .url(url)" to "case .url(let url)": Consistent with UseLetInEveryBoundCaseVariable and matches modern Swift style.
  • Header escaping fix in header.sh: Escaping quotes in the MIT license template is a real correctness fix.
  • Removing Tests from SwiftLint excludes: Tests will now be linted, which is a meaningful quality improvement.

Summary

The tooling migration (Mint to mise) and code quality improvements are well-structured. The two blocking items are the hardcoded branch name in CI (will silently break post-merge) and the significant loss of cross-platform CI coverage (which may be intentional but must be explicitly acknowledged). The stale doc comments and lint script safety gap are lower-priority but worth fixing before the stricter rules surface them.

leogdion added 5 commits June 20, 2026 16:37
…lone repos

Extend ButtondownKit/Spinetail/SwiftTube/Contribute/SyndiKit standalone CI to
the BrightDigit multi-platform template now that a self-hosted macOS runner with
/Applications/Xcode-beta.app (Swift 6.4) is available:

- build-macos: [self-hosted, macOS] + swift-build xcode=Xcode-beta. Blocking on
  all 5. Contribute & SyndiKit migrated off macos-15; their Ubuntu + lint legs
  migrated swift:6.3-noble -> swiftlang/swift:nightly-6.4.x-noble.
- build-windows: hosted windows-2022/2025, swift.org nightly snapshot
  6.4.x-DEVELOPMENT-SNAPSHOT-2026-06-01-a. Gated to full-matrix runs via a
  single-package configure job. continue-on-error on the OpenAPI repos +
  Contribute (unverified deps); blocking on SyndiKit.
- SyndiKit also gains build-macos-platforms (iOS/watchOS/tvOS on released
  Xcode_26.4 — not nightly 6.4).

WASM + Android deferred: no nightly 6.4 support yet. WASM is a swift-build
limitation (brightdigit/swift-build#115 — no input to override the auto-derived
-RELEASE wasm SDK URL); Android is blocked upstream (no nightly 6.4 SDK). Each
workflow documents re-adding them. Contribute WASM is permanently N/A (Yams on
the Musl/wasm SDK).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-bundle inputs

Now that swift.org publishes nightly 6.4 SDK bundles on swift-6.4.x-branch and
brightdigit/swift-build#116 adds inputs to install caller-supplied bundles, add:

- build-wasm to ButtondownKit/Spinetail/SwiftTube/SyndiKit (NOT Contribute — Yams
  fails on the Musl/wasm SDK). Uses wasm-sdk-url + wasm-sdk-checksum pointing at the
  swift-6.4.x-DEVELOPMENT-SNAPSHOT-2026-06-15-a_wasm artifactbundle, with WASI
  emulation + memory flags.
- build-android to all 5 repos. Uses android-sdk-url + android-sdk-id (+ matching
  android-swift-version) for the swift-6.4.x-DEVELOPMENT-SNAPSHOT-2026-06-15-a_android
  artifactbundle via skiptools custom-sdk-url; build-only (android-run-tests: false).

Both legs are full-matrix-gated and continue-on-error: they reference @v1 and are
inert until swift-build#116 is released and the v1 tag moved, after which they
should be confirmed green and promoted to blocking (SyndiKit first). Bump the
snapshot SDK URLs/checksums periodically.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ift 6.4)

Per the rule that Swift 6.4 builds use the self-hosted runners (the only macOS
Swift 6.4 toolchain is /Applications/Xcode-beta.app = Xcode 27 / Swift 6.4):

- Move SyndiKit + Contribute build-macos-platforms off hosted macos-26/Xcode_26.4
  (which is only Swift 6.2) onto [self-hosted, macOS] + Xcode-beta.
- Add build-macos-platforms (iOS/watchOS/tvOS) to ButtondownKit/Spinetail/SwiftTube
  on the same self-hosted runner — these are swift-tools-version:6.4, so hosted
  released-Xcode runners can't even parse their manifests.

All use the iOS/watchOS/tvOS 27.0 simulator runtimes present on the runner
(iPhone 17 Pro / Apple Watch Ultra 3 (49mm) / Apple TV 4K (3rd generation)),
full-matrix-gated and continue-on-error (simulator-on-nightly is the most fragile
leg; promote once green). lint now needs build-macos-platforms.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three CI tiers instead of two: small set (build-ubuntu/build-macos/lint, always)
< full-matrix (macОS-platforms/wasm/android) < +Windows. configure now emits a
second output, run-windows, equal to full-matrix MINUS PRs into semver branches.
build-windows (the most expensive leg — hosted windows-2022/2025 ×2) gates on
run-windows; the rest keep full-matrix. So PRs into semver branches still exercise
the full non-Windows matrix but skip Windows.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nto build-ubuntu

- Point every brightdigit/swift-build@v1 reference at @sdk-url-checksum-nightly-6.4
  (PR #116) TEMPORARILY so CI exercises the new wasm-sdk-url/android-sdk-url code
  before it ships in @v1. Revert to @v1 once #116 is merged + the v1 tag moved.
- Fold build-wasm into build-ubuntu as a continue-on-error step in the same
  nightly-6.4.x-noble container (one spin-up; runs on every push/PR now, not just
  full-matrix). Removed the standalone build-wasm job from ButtondownKit/Spinetail/
  SwiftTube/SyndiKit (Contribute has no wasm — Yams on Musl/wasm).

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

codecov Bot commented Jun 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.30%. Comparing base (746262a) to head (fcae72b).

Additional details and impacted files
@@            Coverage Diff             @@
##           v1.0.0     #127      +/-   ##
==========================================
+ Coverage   77.65%   78.30%   +0.64%     
==========================================
  Files          74       74              
  Lines        1553     1553              
==========================================
+ Hits         1206     1216      +10     
+ Misses        347      337      -10     
Flag Coverage Δ
5.10 ?
6.0 ?
6.1 ?
6.2 ?
6.3 ?
ios18.5 ?
ios26.4 ?
jammy ?
macos 78.30% <100.00%> (+1.41%) ⬆️
noble 78.30% <100.00%> (+0.64%) ⬆️
spm 78.30% <100.00%> (+0.64%) ⬆️
swift-6.4 78.30% <100.00%> (?)
tvos18.5 ?
tvos26.4 ?
visionos2.5 ?
visionos26.4 ?
watchos11.5 ?
watchos26.4 ?

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.

leogdion added 2 commits June 20, 2026 18:05
… trigger)

Push events now trigger only on main (+ release tags). Feature/integration
branches like brightdigit-com-260406 get CI through their PRs instead of a
redundant branch-push run.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Windows is now blocking everywhere (dropped continue-on-error from build-windows
  in all 5, not just SyndiKit).
- The only per-package functional difference — the wasm step (Contribute can't
  build Yams on wasm) — is externalized to the ENABLE_WASM repo variable:
  `if: ${{ vars.ENABLE_WASM != 'false' }}`. Set ENABLE_WASM=false on Contribute.
- Normalized all comments to a shared template.

Result: the five workflow files are byte-identical except the `name:` line; the
only behavioral knob is each repo's ENABLE_WASM variable. Filed Contribute#10 to
fix Yams-on-wasm and re-enable it there.

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

claude Bot commented Jun 20, 2026

Copy link
Copy Markdown

Code Review: PR #127 — Integrate brightdigit.com vendored changes + Swift 6.4 CI

Summary: This PR syncs vendored changes from the brightdigit.com monorepo, migrates the CI pipeline to Swift 6.4 nightly using mise instead of Mint, tightens SwiftLint/swift-format rules, and improves test quality. 145 files changed (+1108/-1046).


🚨 Breaking Changes

Package.swift: macOS minimum raised from .v10_15.v13

This is a semver-breaking change that drops support for macOS Catalina (10.15), Big Sur (11), and Monterey (12). Users on those systems will no longer be able to use this package. This change should be clearly called out in release notes and warrants a major version bump per SemVer.

Additionally, the SyndiKit.docc/SyndiKit.md documentation still references the old deployment targets:

- iOS 15.4 / watchOS 8.5 / tvOS 15.4 / macOS 12.3 or later deployment targets  (old, in diff at line 4143)
+ - iOS 13 / watchOS 6 / tvOS 13 / macOS 10.15 or later deployment targets      (new, but still wrong — should be macOS 13)

The new documentation line is still incorrect — it says macOS 10.15 but Package.swift now requires macOS 13.


⚠️ CI Coverage Regressions

The new workflow significantly narrows test coverage:

Dimension Old New
Ubuntu Swift versions 5.10, 6.0, 6.1, 6.2, 6.3 6.4 nightly only
Windows Swift versions 6.1-RELEASE, 6.2-RELEASE, 6.3-RELEASE 6.4 nightly only
Android API levels 28, 33, 36 (with tests) 34 only (no tests)
visionOS ✅ Tested ❌ Removed
Swift source compat suite swift:6.0–6.3 containers ❌ Removed entirely

Testing exclusively on a nightly snapshot (nightly-6.4.x-noble) is risky: snapshots can break APIs or introduce regressions that releases wouldn't have, and narrowing to one Swift version misses real-world backwards-compatibility issues for downstream users on older toolchains.

Self-hosted runner dependency: macOS builds now require [self-hosted, macOS] runners. This blocks contributors from forking and running CI, which hurts open-source ergonomics. The previous workflow used public GitHub runners (macos-14, macos-15, macos-26).


⚠️ SwiftLint Rule Changes Worth Noting

File length error threshold lowered 550 → 300 while the warning stays at 225. This is tighter than the CLAUDE.md documents (which still says 225/550) and will cause existing files already between 300–550 lines to fail. CLAUDE.md should be updated to reflect the new limits.

one_declaration_per_file rule added. This could break existing files that have multiple declarations and weren't touched in this PR. A lint pass over the whole codebase should be verified before merging.

iTunes type name exclusions removed from type_name. The CLAUDE.md explicitly documents these exclusions (iTunesDuration, iTunesEpisode, iTunesOwner, iTunesImage) as intentional. If SwiftLint now flags them, downstream lint failures will occur.


⚠️ Scripts/lint.shset -e removed

# Remove set -e to allow script to continue running
# set -e  # Exit on any error

The comment says "allow script to continue running" but the manual ERRORS counter only increments on failure — it does not replicate the semantics of set -e for all commands (e.g., unquoted variables, subshell issues). This is a behavior change worth documenting explicitly.

Also note pushd $PACKAGE_DIR (unquoted variable) — if $PACKAGE_DIR contains spaces, this will fail without set -e to catch it.


✅ Positive Changes

Test quality improvements are excellent:

  • Force-unwraps replaced with try XCTUnwrap() throughout — catches test setup failures properly rather than crashing
  • Repeated long URL strings extracted to private static constants (hostImageURLString, guestImageURLString)
  • youtubeID(from:) helper method extracts repeated pattern-match logic from the test body
  • XCTFail() calls now include diagnostic messages (e.g., XCTFail("Expected AtomFeed for \(name)"))
  • [Int]($0.map { $0 }.reversed())Array($0.reversed()) — cleaner, idiomatic Swift
  • case let .pattern(x) = ycase .pattern(let x) = y — correct Swift 5.9+ binding syntax

SwiftLint additions that improve safety:

  • no_unchecked_sendable custom rule is a strong addition — enforces proper Sendable conformance in a Swift 6 concurrency world
  • NeverForceUnwrap, NeverUseForceTry, NeverUseImplicitlyUnwrappedOptionals all enabled in swift-format

header.sh now skips CI: Running the header-rewrite script only locally (not in CI) is correct — it was a latent CI risk before.

Mise migration: Replacing Mint with mise is a reasonable modernization, and the .mise.toml is clear. The lint script's mise detection logic (checking known paths before falling back to command -v) is solid.


Minor Nits

  • .swift-format inconsistency: FileScopedDeclarationPrivacy rule is disabled (false) but accessLevel is still set to fileprivate. The disabled rule means the accessLevel setting is never applied — this is harmless but confusing.
  • Codecov regression: Old CI had fail_ci_if_error: true; new CI has fail_ci_if_error: false. This silently ignores Codecov upload failures.
  • WASM continue-on-error: true with no result check is acceptable for an experimental leg, but the comment mentioning it should be validated is a bit aspirational without a tracking issue.

Summary

The test improvements and tooling modernization are solid. The main concerns before merging are:

  1. The macOS 13 minimum is a breaking change — confirm this is intentional and bump the major version
  2. Fix the documentation in SyndiKit.docc/SyndiKit.md to accurately state the new minimum (macOS 13, not 10.15)
  3. Verify lint passes with the new one_declaration_per_file rule and the tighter file-length limit before merging
  4. Confirm CI coverage reduction is intentional — dropping Swift 5.x/6.x compatibility testing and visionOS/Android tests could mask regressions for downstream users

🤖 Generated with Claude Code

Make the WASM step, the Apple-platforms job (iOS/watchOS/tvOS), and the
Android job blocking across all five subrepos. Tidy the now-stale
continue-on-error comment phrases.

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

claude Bot commented Jun 20, 2026

Copy link
Copy Markdown

Code Review — PR #127: Integrate brightdigit.com vendored changes + Swift 6.4 CI

Overview

This PR syncs the monorepo-vendored copy of SyndiKit back to its standalone repo and upgrades CI to Swift 6.4. It touches 145 files, but most changes are mechanical (license header quote fixes, case let .x(y)case .x(let y), explicit internal modifiers). The substantive changes are in CI, tooling, and linting configuration.


Issues

Breaking: macOS minimum version bump (Package.swift)

- .macOS(.v10_15),
+ .macOS(.v13),

This drops support for macOS 10.15, 11, and 12 — a significant breaking change for anyone running SyndiKit on those systems. Since the repo already has #if swift(<6.0) guards and @available(macOS 13.0, *) scattered throughout, this may be intentional, but it deserves explicit acknowledgement in the PR body and a CHANGELOG entry. Consider whether this belongs in a semver major bump.

CI no longer tests older Swift versions

The old matrix tested Swift 5.6 through 6.3 across Ubuntu jammy and noble. The new workflow targets only Swift 6.4 nightly. The source still contains #if swift(<6.0) guards, so older toolchains are expected to be supported — but there's no CI to verify them. If older Swift support is intentional, at minimum one stable release (e.g. 6.1 or 6.2) should be included in the matrix.

Self-hosted macOS runners are a reliability concern

runs-on: [self-hosted, macOS]

The macOS and platform jobs now require a self-hosted runner with /Applications/Xcode-beta.app hardcoded. If this runner is unavailable or the path changes, those jobs silently fail. Consider a fallback comment or a required check so PR mergeability is preserved.

lint.sh: pushd with unquoted variable

pushd $PACKAGE_DIR

If PACKAGE_DIR ever contains spaces, this will break. Use pushd "$PACKAGE_DIR" consistently (the rest of the script already quotes this variable).

lint.sh: run_command behavior change

The old script had:

if [ "$LINT_MODE" = "STRICT" ]; then
    "$@" || ERRORS=$((ERRORS + 1))
else
    "$@"
fi

The new one always increments ERRORS on failure, regardless of LINT_MODE. This means non-STRICT runs now also exit 1 if any tool fails, which is a behavior change. The current code appears correct for the intended design, but this is worth confirming.

Windows snapshot pins will rot

build: 6.4.x-DEVELOPMENT-SNAPSHOT-2026-06-01-a

As the comment notes, swift.org GCs old snapshots. This URL will stop working in a few weeks. A tracking issue or calendar reminder to bump this periodically would help prevent silent Windows CI failures.

@unchecked Sendable suppression is correct but incomplete

The new no_unchecked_sendable rule fires at error level, and JSONDecoder.swift correctly suppresses it for the #if swift(<5.7) retroactive conformance. But XMLDecoder.swift has the same pattern — double-check that it also has the suppression (or doesn't need it if XMLDecoder already conforms via a different path).


Suggestions

one_declaration_per_file rule — This is an unusual rule for Swift. Extensions in their own files are idiomatic in this codebase (WordPressPost+Init.swift, etc.), and a type + its extension in the same file would violate this rule. Verify it doesn't generate unexpected violations before merging, or add targeted suppressions where needed.

WASM gatingENABLE_WASM defaults to enabled. Since SyndiKit uses XMLCoder (which itself has WASM support questions), confirm the WASM SDK URL and memory flags (--initial-memory=536870912) are appropriate for the package's test suite size.

try! dual-suppression in test support — The pattern of adding both // swift-format-ignore: NeverUseForceTry and // swiftlint:disable:next force_try on every static initializer is noisy. Since these are all in test support code, a file-level // swiftlint:disable force_try at the top of Content.ResultDictionary.swift would be cleaner.


Positives

  • Stricter lint rules (missing_docs, NeverForceUnwrap, NeverUseForceTry) will improve long-term code quality.
  • Renaming c1/c2category1/category2, t1/t2tag1/tag2 is a clear improvement.
  • - Throws: doc added to SynDecoder.decode(_:) is good.
  • Fixed the syntax bug in SyndiKit.md (.video(.youtube(let youtube) missing closing paren).
  • "OPML Feed Formate" typo fixed.
  • LICENSE link fixed to point to the actual blob path.
  • header.sh quote escaping fix is correct and was a real shell bug.
  • Migrating from Mint to mise aligns with the broader Swift ecosystem trend.

Test Coverage

No new tests added, but the linting changes that now include the Tests directory mean existing tests will be checked for violations. The // swiftlint:disable file_length and // swiftlint:disable:this file_name suppressions added to test files are appropriate given those files predate the rules.


Bottom line: The CI and tooling modernization is solid. The main concerns are the undocumented macOS version breaking change, the loss of multi-Swift-version testing, and a few robustness issues in lint.sh. Recommend addressing the macOS bump documentation and adding at least one stable Swift version to the CI matrix before merging.

🤖 Generated with Claude Code

leogdion added 2 commits June 20, 2026 19:33
watchOS-27 SDK rejects deps that infer an 8.0 deployment target (SwiftPM
#10188, not fixable in-repo). Gate the build-macos-platforms watchOS leg on a
new ENABLE_WATCHOS repo variable (mirrors ENABLE_WASM), keeping the 5 workflows
byte-identical. ENABLE_WATCHOS=false is set on ButtondownKit/SwiftTube/
Contribute/Spinetail; SyndiKit (unaffected) keeps running watchOS.

Tracked in brightdigit.com#119.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mirror brightdigit/MistKit: the configure job emits ubuntu-type (gated by
ENABLE_WASM), and build-ubuntu fans out over it via fromJSON. Standard ('')
always runs; wasm + wasm-embedded run unless ENABLE_WASM=false (e.g. Yams on
Contribute). Both wasm variants share the nightly-6.4.x SDK bundle. Replaces the
single folded WASM step. Byte-identical across all 5.

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

claude Bot commented Jun 21, 2026

Copy link
Copy Markdown

Code Review: Integrate brightdigit.com vendored changes + Swift 6.4 CI

Overview: This PR syncs vendored changes from the brightdigit.com monorepo back to the standalone SyndiKit repo and upgrades CI to a Swift 6.4 template. The scope is large (145 files, +1119/-1046) but most changes are mechanical (license header escaping, adding explicit `internal` modifiers, pattern matching style). I've focused on the substantive concerns.


🔴 Breaking Change — macOS Deployment Target

`Package.swift`: `macOS(.v10_15)` → `macOS(.v13)`

This is a semver-major breaking change. Consumers on macOS 10.15 (Catalina), 11, and 12 are silently dropped. For a library package, this should be:

  • Noted prominently in a CHANGELOG or PR description
  • Accompanied by a major version bump when released

If the rationale is Swift 6.4's minimum requirements, that's worth documenting.


🔴 CI Action Version Concerns

Several action versions appear to reference tags that don't yet exist or use non-SHA-pinned references:

  • `actions/checkout@v6` — GitHub's official action is currently at v4. v6 does not exist. This will fail.
  • `codecov/codecov-action@v7` — Currently at v4/v5; v7 does not exist.
  • `jdx/mise-action@v4` — Needs verification.
  • `brightdigit/swift-build@sdk-url-checksum-nightly-6.4` — Branch reference, not a pinned SHA. This is a security risk for a CI pipeline — a branch can be force-pushed.

Recommended: pin all third-party actions to a full commit SHA, not a tag or branch.


🟡 Self-hosted Runner Dependency

Several jobs (`build-macos`, `build-macos-platforms`) require `[self-hosted, macOS]` runners. For an open-source repo:

  • External contributors' PRs won't trigger these jobs unless runners are shared (and sharing self-hosted runners with external PRs is a security concern).
  • Consider documenting which jobs are expected to pass on PRs from forks vs. internal branches.

🟡 SwiftLint iTunes Type Exclusions Removed

The `type_name` rule no longer excludes `iTunesDuration`, `iTunesEpisode`, `iTunesOwner`, `iTunesImage`. These types use the project-convention lowercase-`i` prefix (matching Apple's APIs). Without the exclusion, `type_name` will flag them as violations. Verify linting still passes.


🟡 `missing_docs` Rule Enabled Without Verification

Enabling `missing_docs` means every undocumented `public` API will now be a lint error in `STRICT` mode. This is a good long-term direction, but the PR should confirm all existing public APIs are documented, or linting will fail in CI.


🟡 Nightly Snapshot Staleness

Several snapshots are hardcoded with specific dates:

  • wasm SDK: `swift-6.4.x-DEVELOPMENT-SNAPSHOT-2026-06-15-a`
  • Android SDK: `swift-6.4.x-DEVELOPMENT-SNAPSHOT-2026-06-15-a`
  • Windows: `6.4.x-DEVELOPMENT-SNAPSHOT-2026-06-01-a`

swift.org rotates and eventually removes old snapshots. These will silently break CI unless bumped periodically. Consider adding a comment with a reminder cadence, or a link to where current snapshots can be found.


🟢 Positives

  • `no_unchecked_sendable` custom rule: Excellent addition for Swift 6 strict concurrency. Prevents unsafe shortcuts.
  • Enabled `NeverForceUnwrap`, `NeverUseForceTry`, `NeverUseImplicitlyUnwrappedOptionals` in swift-format: Good for correctness.
  • Variable renaming in tests (`c1`/`c2` → `category1`/`category2`): Improves readability.
  • License header quote escaping fix (`"` → `"`): Correct fix for bash heredoc.
  • Migrating from Mint to mise: mise is more actively maintained and handles multi-backend tool management better.
  • WASM support gated via `ENABLE_WASM` repo variable: Clean feature flag approach.
  • `set -e` removal with error counting: The new error-accumulation pattern in `lint.sh` runs all checks and reports all failures rather than stopping on the first — this is better UX.
  • Added `- Throws:` doc comment to `SynDecoder.decode`: Small but improves DocC output.

Minor Notes

  • Pattern matching style (`case let .x(y)` → `case .x(let y)`): Consistent with Swift 5.9+ direction and the enabled `UseLetInEveryBoundCaseVariable` rule.
  • `accessLevel: fileprivate` in swift-format: This change means file-scoped private declarations will use `fileprivate` instead of `private`. In practice, with one type per file (which `one_declaration_per_file` enforces), there's no behavioral difference, but it's worth verifying the intent.
  • Tests now linted (removed `Tests` from `excluded` in `.swiftlint.yml`): This will surface any style issues in the test suite and is a good change.

Summary: The CI action version issues (`checkout@v6`, `codecov@v7`) need to be resolved before this can merge — the CI will likely fail immediately. The macOS deployment target bump deserves explicit acknowledgment as a breaking change. All other concerns are addressable with minor follow-up.

The wasm/embedded BUILD passes (OpenAPIURLSession is guarded out on WASI), but
running the suite under WasmKit fails: Foundation resource bundles (test
Fixtures) aren't mounted in the WASI sandbox, and embedded hits OpenAPIRuntime
keypath-demangling limits at runtime. Neither is our bug. Build-only validates
wasm/embedded compilation (the goal); the standard leg still runs the full
suite + coverage. Mirrors the Android leg's build-only stance.

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

claude Bot commented Jun 21, 2026

Copy link
Copy Markdown

Code Review: PR #127 — Integrate brightdigit.com vendored changes + Swift 6.4 CI

Overview

This PR syncs changes from a monorepo vendored copy back to the standalone SyndiKit repository, and upgrades the CI pipeline to Swift 6.4. It touches 145 files (+1,123 / -1,046 lines), though the vast majority of those changes are cosmetic.


Change Categories

~80% of the diff is mechanical, non-functional cleanup:

  • License headers across every source file: curly quotes (" ") → straight ASCII quotes (")
  • Import order re-sorted (Foundation before SyndiKit in test targets)
  • Explicit internal access modifiers added to test structs/methods (correct for Swift 6 strict concurrency)
  • Pattern matching style: case let .foo(x)case .foo(let x) (matches swift-format preference)
  • Variable renames for clarity (c1/c2category1/category2, t1/t2tag1/tag2)

These are all correct and appropriate.


CI Workflow Overhaul (.github/workflows/)

Positive:

  • Replaces the verbose 380-line syndikit.yml matrix with a leaner 270-line SyndiKit.yml
  • Smart three-tier matrix logic (small set / full-matrix / +Windows) via the configure job is well-structured
  • WebAssembly and embedded WASM support is a nice addition
  • Android build (API 34 only vs. previous 28/33/34) is a reasonable simplification
  • fail_ci_if_error: false on Codecov upload is correct — never fail CI on a coverage service outage

Concerns:

  • Self-hosted runner dependency: build-macos and build-macos-platforms now require [self-hosted, macOS] with /Applications/Xcode-beta.app hardcoded. This only works on BrightDigit's internal infra and would block contributors trying to fork/run CI. The old workflow used standard GitHub-hosted macos-14/15/26 runners. Consider keeping a fallback or documenting the requirement clearly.
  • Pin of brightdigit/swift-build@sdk-url-checksum-nightly-6.4: Using a branch ref rather than a pinned SHA is a supply-chain risk — if that branch is force-pushed, all CI jobs silently use new code. Prefer brightdigit/swift-build@<full-sha>.
  • brightdigit/swift-coverage-action@2f8538f... — this one IS pinned by SHA (good), but the explanation for why the upstream is broken (sersoft-gmbh/swift-coverage-action@v5) should be captured in a comment or issue link, not just an inline comment. The macOS leg still uses the unfixed sersoft-gmbh/swift-coverage-action@v5 — is that intentional?
  • actions/checkout@v6 — v6 doesn't exist yet as of mid-2026; this will likely resolve to a tag that may not be what you expect. Pinning to v4 or a SHA is safer.
  • Lint job needs list includes build-windows, but build-windows is conditional (if: needs.configure.outputs.run-windows == 'true'). If Windows is skipped (e.g., a PR into a semver branch), the lint job may be blocked waiting for a job that never starts. Consider adding build-android to the needs list as well for full coverage, or restructure with if: ${{ !cancelled() }} for resilience.

Source Code Changes

Package.swift: macOS minimum raised from .v10_15.v13.

  • This is a breaking change for any downstream consumer still on macOS 10.15–12. Warrants a major/minor version bump and clear release notes. The motivation (presumably Swift 6.4 APIs or concurrency requirements) isn't documented here.

.swiftlint.yml:

  • line_length warning raised from 90 → 108, error from 90 → 200. This significantly relaxes a previously strict rule. The CLAUDE.md doc still says "90 characters (strict)" — that doc should be updated.
  • type_name exclusions for iTunes types (iTunesDuration, iTunesEpisode, etc.) removed. These are now handled inline with swiftlint:disable type_name / swiftlint:enable type_name blocks. This is cleaner but means any future iTunes-prefixed type needs a manual disable block — worth a comment in CLAUDE.md.
  • New no_unchecked_sendable custom rule enforcing @unchecked Sendable is prohibited — good for Swift 6 correctness.
  • Tests removed from excluded paths in SwiftLint. This means SwiftLint will now lint test code. This is correct (strict ACL was already needed) but may surface new warnings on the first run.

Scripts/lint.sh:

  • Migration from Mint to mise is a breaking local dev change for anyone with Mint installed. The error message is clear, but no upgrade path is offered for existing contributors.
  • Removing set -e in favor of manual ERRORS tracking is intentional — this allows the script to collect all linting failures rather than stopping at the first. Good.
  • swift build --build-tests added inside the lint script is unusual — this causes a full build during every lint run, which is slow. Typically lint and build are separate CI steps.
  • Header script now skipped in CI (if [ -z "$CI" ]) — correct, avoids mutating source files during CI.
  • Periphery is now also only run locally (not in CI) — fine for a package of this size, but worth noting that dead-code detection is no longer enforced in CI.

File Extractions (correct CLAUDE.md convention):

  • YouTubeIDProperties extracted from YouTubeID.swiftYouTubeIDProperties.swift
  • SiteCollectionDirectoryBuilder extracted from SiteDirectoryBuilder.swiftSiteCollectionDirectoryBuilder.swift
  • Both follow the project's pattern of splitting types into their own files.

WordPressPost+ValidatedFields.swift: Removed // swiftlint:disable/enable function_body_length suppressions. This implies the init was refactored to fit within 50 lines. Good cleanup — suppressions should be a last resort.

Documentation (SyndiKit.md):

  • Fixed typo: "iTunes-compatabile" → "iTunes-compatible" ✓
  • Fixed broken switch pattern: .video(.youtube(let youtube):.video(.youtube(let youtube)): ✓ (missing closing paren was a real bug in the docs)
  • Corrected Feedable mapping table: siteURL was incorrectly mapped to JSONFeed/title, now correctly JSONFeed/homePageUrl
  • Fixed LICENSE link to use the full blob path ✓
  • Code snippets updated to use try Data(contentsOf:) instead of force-unwrapped Data(contentsOf:)!

Tests:

  • URL(strict:)! force-unwrap in UTF8EncodedURLTests replaced with try #require(...) ✓ — better failure diagnostics
  • Import order consistency (Foundation before SyndiKit) applied throughout test support

Summary of Issues to Address Before Merge

Priority Issue
High actions/checkout@v6 likely doesn't exist — verify or pin to a valid ref
High brightdigit/swift-build@sdk-url-checksum-nightly-6.4 branch ref is a supply-chain risk — pin to SHA
Medium Self-hosted runner requirement for macOS jobs is undocumented and blocks contributors
Medium Lint job may deadlock when build-windows is conditionally skipped
Medium Package.swift macOS(.v13) bump is breaking — needs changelog entry and version bump
Low CLAUDE.md still documents 90-char line limit; now 108/200
Low swift build --build-tests inside lint.sh adds unnecessary build overhead
Info macOS coverage still uses unfixed sersoft-gmbh/swift-coverage-action@v5 — intentional?

Overall Assessment

The content of this PR is broadly correct and well-organized. The file-extraction refactors, documentation fixes, and Swift 6 access-modifier cleanup are all improvements. The main concerns are around CI robustness (action version pinning, self-hosted runner assumptions) and the undocumented breaking platform change in Package.swift.

🤖 Generated with Claude Code

@leogdion leogdion changed the title Integrate brightdigit.com vendored changes + Swift 6.4 CI Migrate to Swift 6.4: mise-based lint tooling, rebuilt CI, and source cleanups Jun 21, 2026
Covers the 9 patch-coverage gaps flagged on PR #127:
- EntryID: init(string:)/init?(_:)/description/Codable for all four cases
- SiteCollectionDirectoryBuilder: init() + directory(fromCollection:)
- PodcastEpisodeProperties.init?(rssItem:) nil branch (no enclosure)
- YouTubeIDProperties.init?(entry:) nil branch (non-YouTube Atom entry)

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

claude Bot commented Jun 21, 2026

Copy link
Copy Markdown

Code Review — PR #127: Migrate to Swift 6.4

This is a comprehensive modernisation PR covering CI, tooling, and source cleanups. The overall direction is sound and the changes are well-motivated. Notes below are organised by area.


CI Workflow (.github/workflows/SyndiKit.yml)

Positives

  • The new configure job is cleaner and the tiered matrix logic (small/full/Windows) is easier to follow than the old version.
  • Pinning the forked swift-coverage-action by commit SHA (2f8538f…) is the right call while upstream issue Update CI for December 2026 #92 is open — makes the workaround explicit and auditable.
  • concurrency.cancel-in-progress: true is preserved.

Concerns

  • Loss of multi-version coverage: The old workflow tested Ubuntu against Swift 5.6–6.3 across noble and jammy. The new workflow runs only nightly-6.4.x-noble. This is a meaningful regression for a public library — consumers on Swift 6.0, 6.1, 6.2, or 6.3 have no CI signal. Consider keeping at least a lightweight matrix on stable releases, or document why nightly-only is acceptable here.

  • swift-source-compat-suite job removed: The old workflow had an explicit Swift 6.x source-compatibility job running against Swift 6.0–6.3. This safety net is gone with no replacement. This is worth restoring even as a non-blocking check.

  • Android test regression: The old workflow ran android-run-tests: true against API levels 28, 33, 36 with three Swift versions. The new workflow is build-only (android-run-tests: false) against a single API level (34). If Android test execution was working, dropping it silently is a regression worth calling out explicitly.

  • actions/checkout@v6: This is an unusually high version number — the latest stable is v4. If this is a private fork/action, that's fine, but it's worth confirming the version is intentional.

  • build-ubuntu missing needs: configure in if check: build-macos runs unconditionally (no needs: configure) which is correct since it doesn't depend on the matrix, but it also won't be skipped by ci skip if the commit message contains it — there's an asymmetry with the other jobs.

  • Lint job needs includes build-windows: The lint job lists build-windows as a dependency. Since Windows only runs conditionally (run-windows == 'true'), the lint job will be blocked when Windows is skipped. The !cancelled() && !failure() guards help here, but the dependency edge is fragile — if build-windows is never created (skipped via if:), some versions of GitHub Actions will not correctly resolve the dependency. Verify this works on PRs that don't trigger the Windows leg.


Tooling (.mise.toml, Scripts/lint.sh)

Positives

  • Replacing Mint with mise is a reasonable modernisation — better cross-platform support and no Mint bootstrap ceremony.
  • Accumulating errors instead of early-exiting (set -e removed) is a real improvement for developer UX — you see all failures at once.
  • Restricting header.sh and periphery to local-only runs in CI makes sense and avoids spurious file mutations during CI.

Concerns

  • pushd $PACKAGE_DIR without quoting: If $PACKAGE_DIR contains spaces, this will fail. Use pushd "$PACKAGE_DIR" (and similarly for other unquoted uses like $TOOL_CMD swift-format …).

  • LINT_MODE=INSTALL exits before mise install: The INSTALL check exits before bootstrapping tools. If the intent is "install tools and exit", mise install should run before the exit. As written, INSTALL mode does nothing.

  • run_command no longer respects LINT_MODE: The old run_command only incremented ERRORS in STRICT mode; in non-strict it let failures pass silently. The new version always increments ERRORS. This is actually better behaviour, but the change in semantics is undocumented and could break callers who relied on the old lax mode.

  • Missing Mintfile removal: The Mintfile is still present in the repo (used by the old CI) but the new tooling has moved to .mise.toml. The Mintfile should be removed or updated to avoid confusion — new contributors will wonder which one is authoritative.


Swift Configuration (.swift-format, .swiftlint.yml)

Positives

  • Enabling NeverForceUnwrap, NeverUseForceTry, NeverUseImplicitlyUnwrappedOptionals significantly improves safety guarantees.
  • The no_unchecked_sendable custom rule is well-thought-out.
  • The scoped suppression on webMaster with a clear rationale comment is exactly the right approach for spec-mandated identifiers.
  • Removing inclusive_language from disabled rules (it was globally disabled) and replacing it with a targeted swiftlint:disable around the webMaster field is much better than a blanket suppression.

Concerns

  • FileScopedDeclarationPrivacy disabled but fileScopedDeclarationPrivacy.accessLevel changed to fileprivate: The rule is now disabled (false) in the rules list while the policy is changed to fileprivate. This is contradictory — the formatter will silently apply the fileprivate policy without enforcing it via the rule. Clarify the intent: if you want fileprivate at file scope, enable the rule too.

  • Relaxed thresholds without tests: cyclomatic_complexity error threshold raised from 9 to 12, file_length error from 550 to 300, function_body_length warning removed and error raised from 50 to 76, line_length raised from 90/90 to 108/200. These are significant relaxations. The PR description says they're to accommodate the Swift 6.4 migration — if so, a follow-up to re-tighten after cleanup would be worth tracking.

  • Tests removed from excluded: Previously Tests/ was in the excluded list, so SwiftLint didn't check test files. It's now missing from the exclusion list, which means SwiftLint will now check test code. This might cause unexpected failures if tests use patterns (e.g. XCTAssertNotNil(try …) patterns, force-unwraps in test helpers) that trigger the new strict rules.

  • type_name exclusions removed: The iTunesDuration, iTunesEpisode, iTunesOwner, iTunesImage exclusions for type_name and file_name are gone. This is a breaking change for those type names unless they were also renamed — but they don't appear to be renamed in the diff. Will these now trigger lint errors?


Source Changes

Positives

  • SiteCollectionDirectoryBuilder extracted to its own file — good, follows the one_declaration_per_file rule being enabled.
  • PodcastEpisodeProperties extracted similarly — consistent.
  • case letcase .foo(let bar) pattern updates are correct for Swift 6.4 UseLetInEveryBoundCaseVariable.
  • Removing // swiftlint:disable function_body_length comments from RSSItem+InternalInit.swift and ProcessedBlogsResult.swift is clean — those suppressions are no longer needed with the relaxed threshold.

Minor issues

  • SiteCollectionDirectoryBuilder.swift doc comment: The doc comment on directory(fromCollection:) starts with /// A struct representing an Atom category. — this appears to be copy-paste from another file and is clearly wrong for a SiteDirectoryBuilder method. Same issue appears in PodcastEpisodeProperties.swift (init?(rssItem:) docs start with /// A struct representing an Atom category.).

  • SiteCategory.swift — the - Returns: line was removed from the init?(languages:) doc comment. For a failable initialiser, documenting the nil return condition is valuable. Consider adding it back as /// - Returns: A new ``SiteCategory``, or niliflanguages is empty.

  • SynDecoder.swift — the - Throws: doc line was added inline above the existing prose paragraph, breaking the standard DocC order (Summary → Parameters → Returns → Throws). Minor but worth fixing for DocC rendering.

  • Package.swift macOS minimum raised to 13: This is a significant deployment target bump (from 10.15). Callers on macOS 10.15–12 will need to update. If this is intentional (driven by the @available(macOS 13.0, *) on JSONDecoder conformance), it should be explicitly called out in the PR description and release notes.


Summary

The migration work is solid and the rationale is clear. The main things to address before merging:

  1. Coverage regression: Multi-version Swift testing is gone. At minimum, document the decision or keep a minimal stable-Swift leg.
  2. .mise.toml / Mintfile co-existence: Remove or update Mintfile to avoid confusion.
  3. type_name exclusions: Confirm iTunesXxx names no longer trigger lint errors, or restore the exclusions.
  4. Stale doc comment fragments in SiteCollectionDirectoryBuilder and PodcastEpisodeProperties ("A struct representing an Atom category.").
  5. Shell quoting in lint.sh: Quote $PACKAGE_DIR and other variables.

🤖 Generated with Claude Code

….4 branch deleted)

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

claude Bot commented Jun 21, 2026

Copy link
Copy Markdown

Code Review — PR #127: Migrate to Swift 6.4 / mise tooling / rebuilt CI

This is a well-structured migration PR. The changes are logically grouped, the commit message is detailed, and the intent is clear. Below are observations organized by severity.


🔴 Critical Issues

1. lint job may be silently skipped on non-full-matrix runs

The old CI used always() in the lint if: condition to ensure lint ran even when conditional needed-jobs were skipped. The new CI removes always():

# Old (safe)
if: ${{ always() && !cancelled() && !failure() }}
needs: [build-ubuntu, build-macos, build-windows, build-android]

# New (risky)
if: ${{ !cancelled() && !failure() && ... }}
needs: [build-ubuntu, build-macos, build-windows, build-macos-platforms]

build-windows and build-macos-platforms are conditional jobs (they skip on feature-branch pushes that aren't targeting main/semver). In GitHub Actions, when a needed job is skipped, dependent jobs are also skipped unless always() appears in the if: condition. Without it, lint will be silently skipped on the majority of branch pushes—the exact runs where catching issues early matters most.

Fix: Restore always():

if: ${{ always() && !cancelled() && !failure() && !contains(github.event.head_commit.message, 'ci skip') }}

🟠 Notable Concerns

2. macOS minimum deployment target bumped from 10.15 → 13 (breaking change)

This is a 3-major-version drop that removes support for macOS Monterey (12), Big Sur (11), and Catalina (10.15). Downstream consumers targeting those versions will get a broken build with no warning. The PR description doesn't flag this as a breaking change. Consider a minor version bump or at least a note in the PR/changelog.

3. Swift source compatibility suite removed

The old CI explicitly tested swift build against Swift 6.0, 6.1, 6.2, and 6.3 stable releases via swift-source-compat-suite. That entire job is gone. The new CI targets only the Swift 6.4 nightly (swiftlang/swift:nightly-6.4.x-noble). Consumers pinned to Swift 6.3 (a stable release) now have no CI coverage. At minimum, re-adding a single Swift 6.3 leg or the source-compat suite for stable releases would preserve backward-build confidence.

4. Android tests changed from android-run-tests: truefalse

The old CI actually ran Android tests (3 Swift versions × 3 API levels). The new one is build-only (android-run-tests: false) at a single API level (34). This is a silent regression in test coverage for Android consumers.

5. visionOS removed from CI

The old CI included visionOS builds. It's not mentioned in the PR description as a conscious removal—if this is intentional, it should be called out.


🟡 Quality Observations

6. Doc comment copy-paste error in SiteCollectionDirectoryBuilder

SiteCollectionDirectoryBuilder.swift has an incorrect leading doc comment:

/// A struct representing an Atom category.
///   Creates a site collection directory from a site collection.

"A struct representing an Atom category" does not describe this type. The same malformed boilerplate appears in SiteLanguage.swift and PodcastEpisode.swift's extracted file. These look like a carried-forward template artifact. With ValidateDocumentationComments: true now enabled in .swift-format, the formatter may not catch wrong content—just wrong structure. Worth cleaning up manually.

7. FileScopedDeclarationPrivacy inconsistency in .swift-format

The config sets fileScopedDeclarationPrivacy.accessLevel = "fileprivate" but simultaneously disables the FileScopedDeclarationPrivacy rule ("FileScopedDeclarationPrivacy": false). If the rule is disabled, the formatter will never enforce the access level policy. Either enable the rule (to auto-apply fileprivate) or remove the accessLevel setting. The current state is contradictory.

8. Relaxed SwiftLint thresholds weaken existing quality guardrails

Several thresholds were loosened:

  • Line length: 90 → 108 (warning) / 200 (error)
  • Cyclomatic complexity: max 9 → 12
  • Function body length: 30 → 50 (warning) / 50 → 76 (error)

The old 90-char line limit was strict but enforced readable, reviewable code. 200 chars as an error threshold is extremely permissive. If these are being relaxed to accommodate the new formatter's output, consider whether the existing code actually required these limits to be raised or whether the formatter settings should be adjusted instead.

9. Prerelease swift-format in .mise.toml

"spm:swiftlang/swift-format" = "604.0.0-prerelease-2025-12-17"

Pinning a prerelease is fine for bootstrapping against a nightly toolchain, but it should be noted that this tag may be removed or its behavior may differ from the eventual stable 604.0.0 release. When 604.0.0 stable ships, bump this.


✅ Positive Changes

  • @unchecked Sendable properly scoped: Limiting the retroactive JSONDecoder: @unchecked Sendable conformance to Swift <5.7 only, with an explanatory comment and a targeted swiftlint:disable:next no_unchecked_sendable, is exactly the right approach.
  • Custom no_unchecked_sendable lint rule: Prevents future accidental @unchecked Sendable use while allowing pinpoint suppressions with justification.
  • inclusive_language scoped suppression for webMaster: Correctly explains the RSS 2.0 spec constraint and scopes the disable/enable tightly around the affected declarations.
  • File extraction for one_declaration_per_file: Moving SiteCollectionDirectoryBuilder and PodcastEpisodeProperties into their own files is the right response to enabling this rule.
  • Tests now included in linting: Removing Tests from the SwiftLint exclusion list means test code is now held to the same quality bar.
  • Accumulated error mode in lint.sh: Removing early-exit and accumulating all errors before reporting gives a complete lint picture in a single CI run.
  • case .foo(let x) style: The UseLetInEveryBoundCaseVariable formatter change is a valid Swift style preference; applying it consistently is clean.
  • - Returns: removed from initializer docs: Correct per Swift DocC convention; initializers don't have a return value to document.
  • swift build --build-tests in lint.sh: Catches compilation errors that formatting/linting wouldn't catch.

Summary

The migration direction is sound. The main blockers are the lint job skipping on non-full-matrix runs (#1, CI correctness) and the undocumented breaking macOS deployment target change (#2). The loss of Swift stable-version testing (#3) and Android test execution (#4) are worth revisiting before merging.

🤖 Generated with Claude Code

@leogdion leogdion merged commit c0f3902 into v1.0.0 Jun 21, 2026
14 checks passed
@leogdion leogdion deleted the brightdigit-com-260406 branch June 22, 2026 00:20
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