-
Notifications
You must be signed in to change notification settings - Fork 0
Syndikit swift 6.2 #1
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
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes update several types and closures to conform to the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant DateBox
participant ISO8601DateFormatter
Caller->>DateBox: init?(iso8601: String)
DateBox->>ISO8601DateFormatter: xmlCoderFormatter()
ISO8601DateFormatter-->>DateBox: new formatter instance
DateBox->>ISO8601DateFormatter: parse date string
ISO8601DateFormatter-->>DateBox: Date?
DateBox-->>Caller: DateBox?
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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 (2)
Sources/XMLCoder/Auxiliaries/ISO8601DateFormatter.swift (1)
17-17: Add trailing newline to fix SwiftLint warning.} +Sources/XMLCoder/Encoder/XMLEncoder.swift (1)
42-42: Fix colon spacing and approve Sendable conformance.The
NodeEncodingenum correctly conforms toSendableas it's an enum that should be safe for concurrent access.- public enum NodeEncoding : Sendable { + public enum NodeEncoding: Sendable {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Sources/XMLCoder/Auxiliaries/Box/DateBox.swift(3 hunks)Sources/XMLCoder/Auxiliaries/ISO8601DateFormatter.swift(1 hunks)Sources/XMLCoder/Encoder/DynamicNodeEncoding.swift(1 hunks)Sources/XMLCoder/Encoder/XMLEncoder.swift(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
Sources/XMLCoder/Auxiliaries/Box/DateBox.swift (1)
Sources/XMLCoder/Auxiliaries/ISO8601DateFormatter.swift (1)
xmlCoderFormatter(12-16)
🪛 SwiftLint (0.57.0)
Sources/XMLCoder/Encoder/XMLEncoder.swift
[Warning] 42-42: Colons should be next to the identifier when specifying a type and next to the key in dictionary literals
(colon)
Sources/XMLCoder/Auxiliaries/ISO8601DateFormatter.swift
[Warning] 17-17: Files should have a single trailing newline
(trailing_newline)
🔇 Additional comments (8)
Sources/XMLCoder/Auxiliaries/ISO8601DateFormatter.swift (1)
11-17: LGTM! Good refactoring for concurrency safety.The static factory method approach eliminates potential race conditions from shared global state and aligns well with Swift 6.2's concurrency requirements. Creating a new formatter instance on each call is the safer approach for concurrent usage.
Sources/XMLCoder/Auxiliaries/Box/DateBox.swift (3)
11-11: LGTM! Appropriate Sendable conformance.The
DateBoxstruct correctly conforms toSendableas it's a value type with immutable properties, making it safe for concurrent access.
47-47: Good usage of the new factory method.The replacement of the shared global formatter with
ISO8601DateFormatter.xmlCoderFormatter()is consistent with the concurrency safety improvements.
73-73: Consistent usage of the factory method.The formatter usage here is consistent with the other usage in the file and aligns with the concurrency safety refactoring.
Sources/XMLCoder/Encoder/DynamicNodeEncoding.swift (1)
43-43: LGTM! Appropriate Sendable conformance for protocol.The
DynamicNodeEncodingprotocol correctly conforms toSendableas it only defines static methods and has no mutable state, making it safe for concurrent usage.Sources/XMLCoder/Encoder/XMLEncoder.swift (3)
16-16: LGTM! Appropriate Sendable conformance.The
OutputFormattingstruct correctly conforms toSendableas it's anOptionSetwith immutable properties, making it safe for concurrent access.
236-237: LGTM! Proper @sendable closure annotations.The
@Sendableannotations on the closure typealiases are essential for Swift 6.2 concurrency safety, ensuring these closures can be safely used across concurrency domains.
265-271: Excellent Swift version compatibility handling.The conditional compilation approach elegantly handles the differences between Swift versions while maintaining concurrency safety. In Swift 6.1+, the method can be used directly, while earlier versions require the explicit
@Sendablewrapper.
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.
subrepo: subdir: "Packages/XMLCoder" merged: "2b2a748" upstream: origin: "git@github.com:brightdigit/XMLCoder.git" branch: "syndikit-swift-6.2" commit: "7ba3ef6" git-subrepo: version: "0.4.9" origin: "https://github.com/Homebrew/brew" commit: "23c491e5bc"
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/XMLCoder/Encoder/XMLEncoder.swift (1)
42-42: Fix colon spacing and approve Sendable conformance.The
NodeEncodingenum correctly conforms toSendablesince it's a simple enum with associated values that are inherently sendable. However, there's a spacing issue with the colon.Apply this diff to fix the colon spacing:
- public enum NodeEncoding : Sendable { + public enum NodeEncoding: Sendable {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Sources/XMLCoder/Encoder/XMLEncoder.swift(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
Sources/XMLCoder/Encoder/XMLEncoder.swift (8)
Sources/XMLCoder/Encoder/DynamicNodeEncoding.swift (2)
nodeEncoding(48-50)nodeEncoding(54-56)Tests/XMLCoderTests/NodeEncodingStrategyTests.swift (2)
nodeEncoding(46-48)nodeEncoding(72-74)Tests/XMLCoderTests/AdvancedFeatures/AttributedIntrinsicLegacyTest.swift (4)
nodeEncoding(89-96)nodeEncoding(116-123)nodeEncoding(135-142)nodeEncoding(178-185)Tests/XMLCoderTests/AdvancedFeatures/DynamicNodeDecodingLegacyTest.swift (2)
nodeEncoding(134-139)nodeEncoding(151-158)Tests/XMLCoderTests/AdvancedFeatures/AttributedIntrinsicTest.swift (1)
nodeEncoding(143-150)Tests/XMLCoderTests/EscapedCharactersTest.swift (1)
nodeEncoding(60-62)Tests/XMLCoderTests/AdvancedFeatures/DynamicNodeEncodingTest.swift (3)
nodeEncoding(124-129)nodeEncoding(141-148)nodeEncoding(289-296)Tests/XMLCoderTests/RootLevetExtraAttributesTests.swift (1)
nodeEncoding(55-60)
🪛 SwiftLint (0.57.0)
Sources/XMLCoder/Encoder/XMLEncoder.swift
[Warning] 42-42: Colons should be next to the identifier when specifying a type and next to the key in dictionary literals
(colon)
🔇 Additional comments (3)
Sources/XMLCoder/Encoder/XMLEncoder.swift (3)
16-16: Good addition of Sendable conformance for thread safety.The
OutputFormattingstruct now conforms toSendable, which is appropriate since it's anOptionSetwith immutable properties that can be safely shared across concurrency boundaries.
236-237: Excellent use of @sendable for closure type safety.The typealiases for
XMLNodeEncoderClosureandXMLEncodingClosureare properly updated to use@Sendable, ensuring that closures of these types can be safely passed across concurrency boundaries. This is essential for Swift 6.2 strict concurrency checking.
265-271: Smart compiler version conditional for Sendable handling.The implementation correctly handles the difference between Swift 6.1+ and earlier versions:
- Swift 6.1+: Direct return of
dynamicType.nodeEncoding(for:)method reference- Earlier versions: Explicit
@Sendableclosure wrapperThis approach maintains backward compatibility while leveraging improved method reference handling in newer Swift versions.
The logic is sound because:
- In Swift 6.1+, method references like
dynamicType.nodeEncoding(for:)automatically inherit sendability from their context- In earlier versions, the explicit
@Sendableannotation ensures the closure meets the sendability requirements- The relevant code snippets confirm that
nodeEncoding(for:)implementations returnXMLEncoder.NodeEncodingvalues, which are nowSendable
Adding Support for Swift 6 (#1)
Summary by CodeRabbit
New Features
Sendableprotocol.Refactor
@Sendablefor safer concurrent use.Chores