-
Notifications
You must be signed in to change notification settings - Fork 0
Prepared package for Swift 6.2 #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughUpdates CI Xcode and macOS runner versions; bumps Swift toolchain and formatter/linter pins; migrates SwiftFormat keys to kebab-case and adjusts SwiftLint rules; renames many test methods (removing "test" prefix) and converts TaskTimeLimitTests from a struct to an enum; minor README and doc-comment edits. Changes
Sequence Diagram(s)(omitted — changes are configuration and renames, not a runtime control-flow feature) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Sources/PrincipleConcurrency/SingleUseTransfer.swift (1)
26-26: Consider removing the empty documentation comment line.The standalone
///marker doesn't add value within the usage example's code block. If this was unintentional, consider removing it for cleaner documentation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.github/workflows/pull-request.yml(3 hunks).github/workflows/release.yml(1 hunks).mise.toml(1 hunks).swift-version(1 hunks).swiftformat(1 hunks).swiftlint.yml(8 hunks)Package.swift(2 hunks)README.md(1 hunks)Sources/PrincipleConcurrency/SingleUseTransfer.swift(1 hunks)Tests/PrincipleCollectionsTests/SequenceSortedOnTests.swift(1 hunks)Tests/PrincipleCollectionsTests/StringProtocolCapitalizationTests.swift(1 hunks)Tests/PrincipleConcurrencyTests/TaskTimeLimitTests.swift(10 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-28T15:06:57.229Z
Learnt from: NSFatalError
Repo: NSFatalError/Principle PR: 10
File: Tests/PrincipleConcurrencyTests/SingleUseTransferTests.swift:26-38
Timestamp: 2025-04-28T15:06:57.229Z
Learning: `SingleUseTransfer` in PrincipleConcurrency is designed for single use only. The `testTake()` test is specifically checking if the `take` method compiles correctly in an async context, not testing concurrent finalization behavior.
Applied to files:
README.mdSources/PrincipleConcurrency/SingleUseTransfer.swiftTests/PrincipleConcurrencyTests/TaskTimeLimitTests.swift
🧬 Code graph analysis (2)
Tests/PrincipleCollectionsTests/StringProtocolCapitalizationTests.swift (1)
Sources/PrincipleCollections/StringProtocol+Capitalization.swift (1)
uppercasingFirstCharacter(22-25)
Tests/PrincipleCollectionsTests/SequenceSortedOnTests.swift (1)
Sources/PrincipleCollections/Sequence+SortedOn.swift (4)
sorted(26-33)sorted(48-52)sort(71-78)sort(92-96)
🪛 LanguageTool
README.md
[style] ~17-~17: Consider replacing ‘prove to be’ with a shorter or less frequently used alternative.
Context: ...er may reject code that programmers can prove to be safe. SingleUseTransfer shifts the r...
(PROVE_TO_BE_WORDY)
🪛 SwiftLint (0.57.0)
Package.swift
[Warning] 1-1: Header comments should be consistent with project patterns
(file_header)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-and-test (watchos)
- GitHub Check: build-and-test (ios)
- GitHub Check: build-and-test (tvos)
🔇 Additional comments (18)
README.md (1)
17-19: LGTM!The text reflow improves readability while preserving the original meaning. The documentation clearly explains the purpose of
SingleUseTransfer.Tests/PrincipleCollectionsTests/SequenceSortedOnTests.swift (1)
21-21: LGTM!The test method renames follow Swift Testing framework conventions, where the
testprefix is not required. The@Testattribute handles test discovery, and the method names remain clear and descriptive.Also applies to: 27-27, 33-33, 40-40
Tests/PrincipleCollectionsTests/StringProtocolCapitalizationTests.swift (1)
24-24: LGTM!The test method renames are consistent with Swift Testing framework conventions and align with the changes in other test files across the repository.
Also applies to: 31-31
.github/workflows/release.yml (1)
9-9: Verification complete—Xcode 26.1 and macos-26 runner are available.Xcode 26.1 was released on November 3, 2025 and GitHub Actions supports macos-26 as a public preview runner. The configuration is valid.
Package.swift (2)
60-62: Both feature flags are valid and supported in Swift 6.2.NonisolatedNonsendingByDefault is an opt-in feature available in Swift 6.2 and can be enabled with the .enableUpcomingFeature() flag. MemberImportVisibility (SE-0444) is also a valid upcoming feature available to opt into in Swift 6.x feature flags. No issues found.
1-1: Swift tools version 6.2 is current and correct.The latest Swift tools version is 6.2, which aligns with the update in Package.swift and confirms this change reflects the current tooling standard as of November 2025.
.github/workflows/pull-request.yml (1)
15-15: Verified: Xcode 26.1 and macos-26 runner availability.Xcode 26.1 (build 17B55) was released by Apple on November 3, 2025. GitHub Actions supports macos-26 runners as of November 16, 2025 (in public preview). The configuration at lines 15, 19, and 65 is valid and supported.
.swift-version (1)
1-1: Swift 6.2 is officially released and stable for production use.Swift 6.2 was officially released on September 15, 2025. Swift 6.2 is supported in Xcode 26, 26.0.1, and Xcode 26.1 includes Swift 6.2.1. The
.swift-versionconfiguration is appropriate..mise.toml (1)
8-9: Verify compatibility of SwiftLint and SwiftFormat version upgrades against your project configuration.The version jumps are confirmed to include breaking changes:
- SwiftLint 0.62.2 has three breaking changes: artifact bundle internal paths simplified, Swift 6 compiler required to build, and sorted_imports rule behavior changed (adjacent imports now treated as a single group)
- SwiftFormat 0.58.5 includes command-line option name changes (to kebab-case), increased Swift build requirement (5.7+), and various bugfixes and behavior tweaks that may affect formatting output
The sorted_imports rule change and SwiftFormat formatting behavior tweaks could affect your codebase. Test these versions against your project to ensure the linting and formatting output align with your expectations.
.swiftlint.yml (5)
10-10: Good additions for Swift 6.2 compliance.These newly enabled rules align well with Swift 6.2 best practices, enforcing consistent async usage, modifier ordering, and modern Swift idioms.
Also applies to: 70-70, 81-81, 94-95
170-170: LGTM!Adding
dx,dy,dzto identifier exclusions is appropriate for coordinate delta variables commonly used in mathematical and graphics contexts.
59-59: Consider enablingincompatible_concurrency_annotationfor Swift 6.2.This rule helps detect concurrency annotation compatibility issues, which becomes increasingly important with Swift 6's stricter concurrency model. Having it disabled (commented) might miss potential concurrency bugs.
Is there a specific reason this rule is kept disabled for Swift 6.2? If not, consider enabling it to catch concurrency-related issues early.
192-192: Improved regex precision.Adding the explicit trailing whitespace requirement (
\\s) to the Actor and Sendable attribute patterns makes the regex more precise and reduces false positives.Also applies to: 196-196
178-180: Configuration is reasonable and appropriate.SwiftLint's no_magic_numbers rule ships with allowed_numbers set to [0.0, 1.0, 100.0] by default. The configuration at lines 178-180 includes
[0.0, 1.0, 2.0, 100.0], which expands the defaults by adding 2.0—a common multiplier in software development. These values are appropriate for most Swift projects and represent standard constants (identity elements, simple multipliers, and percentages). No overly restrictive constraints were identified.Tests/PrincipleConcurrencyTests/TaskTimeLimitTests.swift (3)
12-12: Good use of enum as a namespace.Converting from
structtoenumprevents instantiation and makes it clear thatTaskTimeLimitTestsserves purely as a namespace for organizing related test groups.
17-17: LGTM - Cleaner test naming with Swift Testing.Removing the "test" prefix is appropriate when using the
@Testmacro from Swift Testing framework, which doesn't require the prefix for test discovery. The resulting names are more concise while remaining descriptive.Also applies to: 26-26, 36-36, 45-45, 59-59
72-72: Consistent naming across test groups.The Timeout group follows the same naming pattern as Deadline, maintaining consistency throughout the test suite.
Also applies to: 81-81, 91-91, 100-100, 114-114
.swiftformat (1)
1-125: No SwiftFormat version found in accessible configuration files.SwiftFormat officially supports kebab-case option names with backward compatibility for existing formats, making the configuration syntactically valid. However, the project's SwiftFormat version could not be identified in
.mise.tomlor other accessible tooling files. To ensure compatibility, verify that your SwiftFormat version (available viaswiftformat --versionlocally) supports all options used in this config, particularly the newer ones like--blank-line-after-switch-case,--default-test-suite-attributes, and--xcode-indentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
README.md (1)
19-21: Consider tightening the phrase "prove to be safe."The refactored text clearly explains SingleUseTransfer's purpose. Minor improvement: the phrase "prove to be safe" can be shortened to "show to be safe" or simply "be safe" for conciseness.
Apply this diff if you'd like to tighten the wording:
-Since Swift currently lacks a built-in annotation to indicate that a closure is guaranteed to be invoked at most once, the compiler may reject code that programmers can prove to be safe. +Since Swift currently lacks a built-in annotation to indicate that a closure is guaranteed to be invoked at most once, the compiler may reject code that programmers can show to be safe.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-28T15:06:57.229Z
Learnt from: NSFatalError
Repo: NSFatalError/Principle PR: 10
File: Tests/PrincipleConcurrencyTests/SingleUseTransferTests.swift:26-38
Timestamp: 2025-04-28T15:06:57.229Z
Learning: `SingleUseTransfer` in PrincipleConcurrency is designed for single use only. The `testTake()` test is specifically checking if the `take` method compiles correctly in an async context, not testing concurrent finalization behavior.
Applied to files:
README.md
🪛 LanguageTool
README.md
[style] ~19-~19: Consider replacing ‘prove to be’ with a shorter or less frequently used alternative.
Context: ...er may reject code that programmers can prove to be safe. SingleUseTransfer shifts the r...
(PROVE_TO_BE_WORDY)
🪛 markdownlint-cli2 (0.18.1)
README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
4-4: Images should have alternate text (alt text)
(MD045, no-alt-text)
9-9: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4
(MD001, heading-increment)
🔇 Additional comments (2)
README.md (2)
12-12: Documentation link in Contents is well-placed.This addition improves readability and navigation by pointing readers to the new Documentation section.
61-63: Documentation section is a valuable addition.This new section effectively directs users to the Swift Package Index documentation. The link placement and formatting are consistent with the rest of the README.
| [](https://swiftpackageindex.com/NSFatalError/Principle) | ||
| [](https://swiftpackageindex.com/NSFatalError/Principle) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add alt text to badge images.
The badges are now dynamic and will auto-update—a solid improvement for maintenance. However, they're missing alt text, which impacts accessibility and triggers linting warnings.
Apply this diff to add alt text:
-[](https://swiftpackageindex.com/NSFatalError/Principle)
-[](https://swiftpackageindex.com/NSFatalError/Principle)
+[](https://swiftpackageindex.com/NSFatalError/Principle)
+[](https://swiftpackageindex.com/NSFatalError/Principle)📝 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.
| [](https://swiftpackageindex.com/NSFatalError/Principle) | |
| [](https://swiftpackageindex.com/NSFatalError/Principle) | |
| [](https://swiftpackageindex.com/NSFatalError/Principle) | |
| [](https://swiftpackageindex.com/NSFatalError/Principle) |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
4-4: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In README.md around lines 3 to 4, the two badge images lack alt text which hurts
accessibility and triggers linters; update the markdown image syntax to include
descriptive alt text for each badge (e.g., "Swift versions badge" and "Platforms
badge") while keeping the existing link targets and URLs unchanged so the badges
remain clickable and dynamic.
Summary by CodeRabbit
Chores
Style
Documentation
Tests