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
Optimise the Swift version of Vexilla Client #42
Conversation
…r depending on this package
Since safeGet is not used, we can add custom error messages that clarify the specific issue at hand. Also by leveraging `#function` you can trace back the origin of the error. Note: String errors should still be migrated over to concrete types. This commit also adopts `guard let` syntax to keep cyclomatic complexity low.
👷 Deploy request for forms-vexilla pending review.Visit the deploys page to approve it
|
👷 Deploy request for auth-vexilla pending review.Visit the deploys page to approve it
|
The "PR Test / test-swift" job seems to be failing. Here is the first error that gets repeated a bunch. /__w/vexilla/vexilla/clients/swift/Sources/VexillaClient/Types.swift:85:33: error: expected ',' separator
public init(from decoder: any Decoder) throws { The second error that repeats a lot with slight variations is: /__w/vexilla/vexilla/clients/swift/Sources/VexillaClient/Types.swift:253:62: error: cannot infer contextual base in reference to member 'value'
let value = try container.decode(String.self, forKey: .value) |
Right, Swift toolchain difference. I forgot about some of these language features having been added more recently. |
…e recent Swift token
Not sure why the tests didn't kick off when you updated it. That is a bug on my end in the workflow. However, after an edit of the description of the PR, I was able to kick it off again and the tests now pass inside of the container. |
@@ -226,7 +226,6 @@ jobs: | |||
steps: | |||
- uses: actions/checkout@v3 | |||
- run: | | |||
cd ./clients/swift |
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.
suggestion (blocking): I would prefer not to move the Package.swift file to the top level. With plans for 14 SDKs, the root level of the project could become unwieldy if all clients SDKs did this.
Does having the Package.swift in a nested folder break something for SwiftPM?
XCTAssertGreaterThan
-family of functions for comparing valuessafeGet
and all code that leveraged this.cmgriffing: editing this description to hopefully kick of the github action (x3)