Integrate brightdigit.com vendored changes + Swift 6.4 CI#3
Conversation
|
Warning Review limit reached
More reviews will be available in 24 minutes and 14 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds ChangesConfigValueReading Protocol
CI and Tooling Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
Move the CLI->ENV->default source-precedence resolution that downstream
consumers hand-wrote as `extension ConfigReader { read(_:) }` into the
Foundation-only core behind a `ConfigValueReading` protocol. An associated
`Key` type mirrors swift-configuration's ConfigReader read surface so a
consumer conforms in one line (`makeConfigKey`), and the read(_:) overloads
(String/Bool/Double required; String/Int/Double/Date optional) are shared
and unit-tested via a mock conformer.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n_per_file) Split MockConfigValueReader into its own file and replace force-unwraps with try #require, so swift-format + SwiftLint --strict pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
leogdion
force-pushed
the
1-remove-need-for-extension
branch
from
f3ebb33 to
e1508de
Compare
Add swiftlang/swift:nightly-6.4 to the main build-ubuntu matrix and the source-compatibility suite. Native target only on the main matrix, since nightly does not publish matching Wasm SDK snapshots. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The release-branch nightly tags use the X.Y.x form, e.g. swiftlang/swift:nightly-6.4.x-noble; nightly-6.4-noble does not exist. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Swift 6.4's swiftbuild engine emits an Xcode-like coverage layout (<Name>Tests.so + -test-runner stub, no .xctest) that upstream sersoft-gmbh/swift-coverage-action@v5 can't pair, so the 6.4 nightly Ubuntu leg failed with "No coverage files found" under fail-on-empty-output: true. Point the Ubuntu coverage step at the SHA-pinned brightdigit fork (Linux-gated patch); macOS legs stay on upstream @v5. See brightdigit.com#111 / issue #92. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ConfigKeyKit.yml (1)
61-74:⚠️ Potential issue | 🟠 MajorPin Swift container images by digest instead of mutable tags.
Line 73 dynamically constructs container references that resolve to mutable tags (
swiftlang/swift:nightly-6.4.x,swift:6.x-{os}) without digest pinning. Line 61 seeds the same unversioned nightly image in the matrix. This weakens CI reproducibility and supply-chain security. Switch container references to@sha256:digests or inject digests through matrix data.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ConfigKeyKit.yml around lines 61 - 74, The container images in the build-ubuntu job are using mutable tags (swiftlang/swift:nightly-6.4.x and swift:6.x-{os}) instead of immutable sha256 digests, which weakens reproducibility and security. Update the matrix configuration where ubuntu-swift is defined (around line 61) to include sha256 digest values alongside the version and image information for each Swift version entry, and then modify the container reference in the build-ubuntu job (around line 73) to construct container references using these digests instead of the mutable tag format calls.Source: Linters/SAST tools
🧹 Nitpick comments (4)
Tests/ConfigKeyKitTests/ConfigValueReadingTests.swift (1)
30-31: 💤 Low valueConsider using explicit import visibility modifiers.
The coding guidelines require explicit
public importorinternal importmodifiers per the InternalImportsByDefault convention. While test files are not part of the public API, applying the convention consistently would align with the project's Swift 6.2 configuration.🔧 Proposed change
-import Foundation -import Testing +internal import Foundation +internal import Testing🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/ConfigKeyKitTests/ConfigValueReadingTests.swift` around lines 30 - 31, The import statements in ConfigValueReadingTests.swift for Foundation and Testing lack explicit visibility modifiers required by the InternalImportsByDefault convention. Add the `internal` modifier to both import statements (before Foundation and Testing) to make the visibility explicit and align with the project's Swift 6.2 configuration and coding guidelines.Source: Coding guidelines
Tests/ConfigKeyKitTests/MockConfigValueReader.swift (1)
35-59: ⚡ Quick winConsider adding
Sendableconformance for concurrency safety.The struct should conform to
Sendablesince it only contains value-type dictionaries and will likely conform to aSendable-constrainedConfigValueReadingprotocol. As per coding guidelines, everything should beSendablefor Swift 6 concurrency safety.🔒 Proposed enhancement
-internal struct MockConfigValueReader: ConfigValueReading { +internal struct MockConfigValueReader: ConfigValueReading, Sendable {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/ConfigKeyKitTests/MockConfigValueReader.swift` around lines 35 - 59, The MockConfigValueReader struct needs to conform to Sendable for Swift 6 concurrency safety. Add Sendable conformance to the struct declaration so that it reads as "internal struct MockConfigValueReader: ConfigValueReading, Sendable". This is safe because the struct only contains value-type dictionaries with String keys and Sendable values (String, Int, Double), and the ConfigValueReading protocol will likely require Sendable conformance.Source: Coding guidelines
Sources/ConfigKeyKit/ConfigValueReading.swift (2)
54-69: ⚡ Quick winConsider adding
Sendableconformance for Swift 6 concurrency safety.As per coding guidelines, everything should be
Sendablefor concurrency safety. Consider constraining theKeyassociated type toSendableand declaring the protocol itself as requiringSendableconformance, since readers may be used across concurrency boundaries.🔒 Proposed enhancement
-public protocol ConfigValueReading { +public protocol ConfigValueReading: Sendable { /// The reader's native key type (e.g. `Configuration.ConfigKey`). - public associatedtype Key + public associatedtype Key: Sendable🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/ConfigKeyKit/ConfigValueReading.swift` around lines 54 - 69, The ConfigValueReading protocol lacks Sendable conformance required for Swift 6 concurrency safety. Add Sendable conformance to the ConfigValueReading protocol declaration by having it inherit from or conform to Sendable, and constrain the Key associated type to also be Sendable. This ensures that implementations of this protocol and their key types are safe to use across concurrency boundaries.Source: Coding guidelines
124-124: ⚡ Quick winCache the ISO8601DateFormatter instance to improve performance.
Creating a new
ISO8601DateFormatter()on each call is inefficient. ISO8601DateFormatter creation has measurable overhead, especially in tight loops or hot paths. Consider using a static cached instance or a thread-safe singleton.⚡ Proposed optimization
+ private static let iso8601Formatter = ISO8601DateFormatter() + /// Reads an optional ISO8601 date value: CLI → ENV → `nil`. public func read(_ key: OptionalConfigKey<Date>) -> Date? { guard let value = resolvedString(key) else { return nil } - return ISO8601DateFormatter().date(from: value) + return Self.iso8601Formatter.date(from: value) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/ConfigKeyKit/ConfigValueReading.swift` at line 124, The ISO8601DateFormatter instance is being created fresh on every call to the function containing the date(from:) method, which causes unnecessary overhead. Cache the ISO8601DateFormatter as a static property at the type level (either in a property wrapper, a separate utility struct, or as a lazy static variable at the module level) and reuse the same instance instead of creating a new formatter on each invocation. Modify the return statement to use the cached formatter instance instead of creating a new one with ISO8601DateFormatter().
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Around line 23-24: The documentation in CLAUDE.md incorrectly references a
protocol named `ConfigurationKey` at line 23, but the actual protocol in the
project is named `ConfigKey`. Update the documentation to replace all instances
of `ConfigurationKey` with `ConfigKey` to ensure the architecture guidance
directs contributors to the correct protocol name and API they should be using.
In `@Sources/ConfigKeyKit/ConfigValueReading.swift`:
- Line 30: Change the import visibility modifier for Foundation from `public
import Foundation` to `internal import Foundation` to comply with the
InternalImportsByDefault convention. The Foundation framework should not be
re-exported publicly from this module, so it requires an explicit internal
import modifier instead of public.
- Around line 54-69: Add explicit public access control modifiers to all
protocol requirements in the ConfigValueReading protocol. Specifically, add the
public keyword before the associatedtype Key declaration and before each method
declaration (makeConfigKey, string, int, and double) to comply with the
explicit_acl and explicit_top_level_acl SwiftLint rules. While the protocol
itself is already marked public, all its members must have explicit access
modifiers as well.
---
Outside diff comments:
In @.github/workflows/ConfigKeyKit.yml:
- Around line 61-74: The container images in the build-ubuntu job are using
mutable tags (swiftlang/swift:nightly-6.4.x and swift:6.x-{os}) instead of
immutable sha256 digests, which weakens reproducibility and security. Update the
matrix configuration where ubuntu-swift is defined (around line 61) to include
sha256 digest values alongside the version and image information for each Swift
version entry, and then modify the container reference in the build-ubuntu job
(around line 73) to construct container references using these digests instead
of the mutable tag format calls.
---
Nitpick comments:
In `@Sources/ConfigKeyKit/ConfigValueReading.swift`:
- Around line 54-69: The ConfigValueReading protocol lacks Sendable conformance
required for Swift 6 concurrency safety. Add Sendable conformance to the
ConfigValueReading protocol declaration by having it inherit from or conform to
Sendable, and constrain the Key associated type to also be Sendable. This
ensures that implementations of this protocol and their key types are safe to
use across concurrency boundaries.
- Line 124: The ISO8601DateFormatter instance is being created fresh on every
call to the function containing the date(from:) method, which causes unnecessary
overhead. Cache the ISO8601DateFormatter as a static property at the type level
(either in a property wrapper, a separate utility struct, or as a lazy static
variable at the module level) and reuse the same instance instead of creating a
new formatter on each invocation. Modify the return statement to use the cached
formatter instance instead of creating a new one with ISO8601DateFormatter().
In `@Tests/ConfigKeyKitTests/ConfigValueReadingTests.swift`:
- Around line 30-31: The import statements in ConfigValueReadingTests.swift for
Foundation and Testing lack explicit visibility modifiers required by the
InternalImportsByDefault convention. Add the `internal` modifier to both import
statements (before Foundation and Testing) to make the visibility explicit and
align with the project's Swift 6.2 configuration and coding guidelines.
In `@Tests/ConfigKeyKitTests/MockConfigValueReader.swift`:
- Around line 35-59: The MockConfigValueReader struct needs to conform to
Sendable for Swift 6 concurrency safety. Add Sendable conformance to the struct
declaration so that it reads as "internal struct MockConfigValueReader:
ConfigValueReading, Sendable". This is safe because the struct only contains
value-type dictionaries with String keys and Sendable values (String, Int,
Double), and the ConfigValueReading protocol will likely require Sendable
conformance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 59aed811-6416-48af-a319-658ddc0fa034
📒 Files selected for processing (6)
.github/workflows/ConfigKeyKit.yml.github/workflows/swift-source-compat.ymlCLAUDE.mdSources/ConfigKeyKit/ConfigValueReading.swiftTests/ConfigKeyKitTests/ConfigValueReadingTests.swiftTests/ConfigKeyKitTests/MockConfigValueReader.swift
| **Config keys** (the core purpose) — A `ConfigurationKey` protocol exposes one method: `key(for: ConfigKeySource) -> String?`. Two concrete value types implement it: | ||
| - `ConfigKey<Value>` — has a required `defaultValue` (resolution is non-optional). |
There was a problem hiding this comment.
Correct the protocol name in architecture guidance.
Line 23 documents ConfigurationKey, but the project contract uses ConfigKey. This will send contributors to the wrong symbol/API naming.
Based on learnings: The ConfigKey protocol has key(for: ConfigKeySource) -> String? as its single required method.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CLAUDE.md` around lines 23 - 24, The documentation in CLAUDE.md incorrectly
references a protocol named `ConfigurationKey` at line 23, but the actual
protocol in the project is named `ConfigKey`. Update the documentation to
replace all instances of `ConfigurationKey` with `ConfigKey` to ensure the
architecture guidance directs contributors to the correct protocol name and API
they should be using.
Source: Learnings
| // OTHER DEALINGS IN THE SOFTWARE. | ||
| // | ||
|
|
||
| public import Foundation |
There was a problem hiding this comment.
Fix import visibility to comply with InternalImportsByDefault.
Foundation should be imported as internal import Foundation, not public import Foundation. As per coding guidelines, the InternalImportsByDefault convention requires explicit internal import or public import modifiers, and Foundation specifically should use internal import.
🔧 Proposed fix
-public import Foundation
+internal import Foundation📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public import Foundation | |
| internal import Foundation |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/ConfigKeyKit/ConfigValueReading.swift` at line 30, Change the import
visibility modifier for Foundation from `public import Foundation` to `internal
import Foundation` to comply with the InternalImportsByDefault convention. The
Foundation framework should not be re-exported publicly from this module, so it
requires an explicit internal import modifier instead of public.
Source: Coding guidelines
| public protocol ConfigValueReading { | ||
| /// The reader's native key type (e.g. `Configuration.ConfigKey`). | ||
| associatedtype Key | ||
|
|
||
| /// Builds a native ``Key`` from a resolved per-source key string. | ||
| func makeConfigKey(_ string: String) -> Key | ||
|
|
||
| /// Reads a string value for the native key, or `nil` if absent. | ||
| func string(forKey key: Key, isSecret: Bool, fileID: String, line: UInt) -> String? | ||
|
|
||
| /// Reads an integer value for the native key, or `nil` if absent. | ||
| func int(forKey key: Key, isSecret: Bool, fileID: String, line: UInt) -> Int? | ||
|
|
||
| /// Reads a double value for the native key, or `nil` if absent. | ||
| func double(forKey key: Key, isSecret: Bool, fileID: String, line: UInt) -> Double? | ||
| } |
There was a problem hiding this comment.
Add explicit access control to all protocol requirements.
The coding guidelines enforce SwiftLint's explicit_acl and explicit_top_level_acl rules, requiring explicit access modifiers on every declaration. Protocol requirements (associatedtype and methods) currently lack explicit public modifiers.
🔧 Proposed fix
public protocol ConfigValueReading {
/// The reader's native key type (e.g. `Configuration.ConfigKey`).
- associatedtype Key
+ public associatedtype Key
/// Builds a native ``Key`` from a resolved per-source key string.
- func makeConfigKey(_ string: String) -> Key
+ public func makeConfigKey(_ string: String) -> Key
/// Reads a string value for the native key, or `nil` if absent.
- func string(forKey key: Key, isSecret: Bool, fileID: String, line: UInt) -> String?
+ public func string(forKey key: Key, isSecret: Bool, fileID: String, line: UInt) -> String?
/// Reads an integer value for the native key, or `nil` if absent.
- func int(forKey key: Key, isSecret: Bool, fileID: String, line: UInt) -> Int?
+ public func int(forKey key: Key, isSecret: Bool, fileID: String, line: UInt) -> Int?
/// Reads a double value for the native key, or `nil` if absent.
- func double(forKey key: Key, isSecret: Bool, fileID: String, line: UInt) -> Double?
+ public func double(forKey key: Key, isSecret: Bool, fileID: String, line: UInt) -> Double?
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/ConfigKeyKit/ConfigValueReading.swift` around lines 54 - 69, Add
explicit public access control modifiers to all protocol requirements in the
ConfigValueReading protocol. Specifically, add the public keyword before the
associatedtype Key declaration and before each method declaration
(makeConfigKey, string, int, and double) to comply with the explicit_acl and
explicit_top_level_acl SwiftLint rules. While the protocol itself is already
marked public, all its members must have explicit access modifiers as well.
Source: Coding guidelines
Make source precedence a stable, documented API instead of relying on the undocumented CaseIterable declaration order. - Add PrioritizedConfigKeySource protocol with `static var priority` (defaults to Array(allCases)); ConfigKeySource pins an explicit [.commandLine, .environment], decoupling precedence from case order. - Add overridable `sourcePriority` to ConfigValueReading (defaults to ConfigKeySource.priority); resolvedString/Int/Double now iterate it instead of allCases. - Tests: pin priority order + drift guard (priority covers every case), and a reversed-sourcePriority override resolving ENV over CLI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/ConfigKeyKit/ConfigValueReading.swift (1)
96-109:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake Bool resolution honor
sourcePriority.Line 97 and Line 102 hard-code CLI→ENV, so overriding
sourcePrioritydoes not affectread(_ key: ConfigKey<Bool>). This breaks the precedence contract used by the other typed reads.Suggested fix
public func read(_ key: ConfigKey<Bool>) -> Bool { - if let cli = key.key(for: .commandLine), - string(forKey: makeConfigKey(cli), isSecret: false, fileID: `#fileID`, line: `#line`) != nil - { - return true - } - if let env = key.key(for: .environment), - let value = string( - forKey: makeConfigKey(env), isSecret: false, fileID: `#fileID`, line: `#line` - ) - { - let normalized = value.lowercased().trimmingCharacters(in: .whitespaces) - return normalized == "true" || normalized == "1" || normalized == "yes" + for source in sourcePriority { + guard let keyString = key.key(for: source) else { continue } + guard let value = string( + forKey: makeConfigKey(keyString), isSecret: false, fileID: `#fileID`, line: `#line` + ) else { continue } + + switch source { + case .commandLine: + return true // flag presence + case .environment: + let normalized = value.lowercased().trimmingCharacters(in: .whitespaces) + return normalized == "true" || normalized == "1" || normalized == "yes" + } } return key.defaultValue }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/ConfigKeyKit/ConfigValueReading.swift` around lines 96 - 109, The `read(_ key: ConfigKey<Bool>)` method currently hard-codes the precedence order as CLI then ENV on lines 97 and 102, ignoring the `sourcePriority` setting. Refactor this method to respect the `sourcePriority` configuration by checking the sources in the order they are defined in `sourcePriority`, just like the other typed read methods do. This ensures the Bool resolution honors the same precedence contract as other typed reads instead of always checking CLI before ENV.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/ConfigKeyKit/ConfigKeySource.swift`:
- Line 39: The extension declaration for ConfigKeySource that conforms to
PrioritizedConfigKeySource is missing an explicit access control modifier. Add
an explicit access modifier (public or internal) before the extension keyword in
the ConfigKeySource extension declaration to comply with the
explicit_top_level_acl SwiftLint rule.
---
Outside diff comments:
In `@Sources/ConfigKeyKit/ConfigValueReading.swift`:
- Around line 96-109: The `read(_ key: ConfigKey<Bool>)` method currently
hard-codes the precedence order as CLI then ENV on lines 97 and 102, ignoring
the `sourcePriority` setting. Refactor this method to respect the
`sourcePriority` configuration by checking the sources in the order they are
defined in `sourcePriority`, just like the other typed read methods do. This
ensures the Bool resolution honors the same precedence contract as other typed
reads instead of always checking CLI before ENV.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9b30ec95-2712-48c6-b539-87fd961a6b09
📒 Files selected for processing (6)
Sources/ConfigKeyKit/ConfigKeySource.swiftSources/ConfigKeyKit/ConfigValueReading.swiftSources/ConfigKeyKit/PrioritizedConfigKeySource.swiftTests/ConfigKeyKitTests/ConfigKeySourceTests.swiftTests/ConfigKeyKitTests/ConfigValueReadingTests.swiftTests/ConfigKeyKitTests/MockConfigValueReader.swift
✅ Files skipped from review due to trivial changes (1)
- Sources/ConfigKeyKit/PrioritizedConfigKeySource.swift
🚧 Files skipped from review as they are similar to previous changes (2)
- Tests/ConfigKeyKitTests/MockConfigValueReader.swift
- Tests/ConfigKeyKitTests/ConfigValueReadingTests.swift
| case environment | ||
| } | ||
|
|
||
| extension ConfigKeySource: PrioritizedConfigKeySource { |
There was a problem hiding this comment.
Add explicit ACL to the extension declaration.
Line 39 introduces a top-level extension without an explicit access modifier. Under explicit_top_level_acl, declare this as public (or internal) explicitly.
Suggested fix
-extension ConfigKeySource: PrioritizedConfigKeySource {
+public extension ConfigKeySource: PrioritizedConfigKeySource {As per coding guidelines, "Declare access control explicitly on every declaration using explicit_acl and explicit_top_level_acl rules enforced by SwiftLint."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| extension ConfigKeySource: PrioritizedConfigKeySource { | |
| public extension ConfigKeySource: PrioritizedConfigKeySource { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/ConfigKeyKit/ConfigKeySource.swift` at line 39, The extension
declaration for ConfigKeySource that conforms to PrioritizedConfigKeySource is
missing an explicit access control modifier. Add an explicit access modifier
(public or internal) before the extension keyword in the ConfigKeySource
extension declaration to comply with the explicit_top_level_acl SwiftLint rule.
Source: Coding guidelines
…ble parsing, isSecret - Bool reads honor sourcePriority (was hardcoded CLI→ENV); empty ENV value is now treated as absent and falls back to the key's default. - Add read(ConfigKey<Int>) and read(OptionalConfigKey<Bool>) to close the required-Int / optional-Bool API gaps. - Add generic read(_:parsing:) that falls through on parse failure; Date becomes a convenience defaulting to ISO8601 with a caller-overridable parser. - Make isSecret a settable property on keys (default false), exposed via the ConfigurationKey protocol, and forward it through every read(). - Collapse the duplicated resolved* helpers into one generic helper. - Tests: reversed-priority bool, empty-ENV bool, date fallthrough, custom date parser, generic parse, isSecret forwarding, and a priority count guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Syncs the brightdigit.com monorepo-vendored copy of this package back to its standalone repository, and brings CI in line with the BrightDigit Swift 6.4 template where applicable.
Also closes #1 (the
ConfigValueReadingextension that removes the need for downstream consumers to hand-write source-precedence resolution) and closes #2 (an explicit, documentedConfigKeySourceprecedence contract).Issue #2 — stable precedence contract
Source precedence (CLI → ENV) was only an implicit side effect of
CaseIterabledeclaration order. This makes it explicit and documented:PrioritizedConfigKeySourceprotocol withstatic var priority(defaults toArray(allCases));ConfigKeySourcepins an explicit[.commandLine, .environment], decoupling precedence fromcaseorder.ConfigValueReadinggains an overridablesourcePriority(defaults toConfigKeySource.priority); resolution loops iterate it instead ofallCases, so a reader can override precedence per-instance.prioritycovers every case), and verify a reversedsourcePriorityresolves ENV over CLI.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
ConfigValueReadingprotocol with priority-based resolution across command-line, environment, and defaults (including ISO8601 optionalDateparsing and boolean normalization).ConfigKeySource.priorityordering as part of the public API.Infrastructure
Documentation
CLAUDE.mdwith repository guidance and common commands.Tests