Skip to content
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

Update SwiftLint to v0.55.1 #290

Merged
merged 1 commit into from
Aug 15, 2024
Merged

Update SwiftLint to v0.55.1 #290

merged 1 commit into from
Aug 15, 2024

Conversation

rebello95
Copy link
Collaborator

Updates our version of SwiftLint, but opts out of one of the new rules because making the required changes:

--- a/Libraries/Connect/Internal/Interceptors/GRPCWebInterceptor.swift
+++ b/Libraries/Connect/Internal/Interceptors/GRPCWebInterceptor.swift
@@ -260,17 +260,11 @@ extension GRPCWebInterceptor: StreamInterceptor {
     }
 }
 
-private struct TrailersDecodingError: Error {}
-
 private extension Trailers {
     static func fromGRPCHeadersBlock(_ source: Data) throws -> Self {
-        guard let string = String(data: source, encoding: .utf8) else {
-            throw TrailersDecodingError()
-        }
-
         // Decode trailers based on gRPC Web spec:
         // https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-WEB.md
-        return string
+        return String(decoding: source, as: UTF8.self)
             .split(separator: "\r\n")
             .reduce(into: Trailers()) { trailers, line in
                 guard let separatorIndex = line.firstIndex(of: ":") else {

Caused some tests to fail:

FAILED: gRPC-Web Unexpected Responses/HTTPVersion:1/TLS:false/trailers-in-body/unary/multiple-responses:
	actual error {code: 2 (unknown), message: "RPC response missing status"} does not match expected code 12 (unimplemented)

It looks like there was some discussion around this rule being potentially problematic here: realm/SwiftLint#5263 (comment)

Signed-off-by: Michael Rebello <[email protected]>
@rebello95 rebello95 requested a review from jhump August 15, 2024 02:31
@rebello95 rebello95 changed the title Update SwiftLint to 0.55.1 Update SwiftLint to v0.55.1 Aug 15, 2024
Copy link
Contributor

@eseay eseay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call on disabling that rule. In some of my other repos, I've gone through the process of migrating to the new format without having the context outlined in that issue commentary, and I now regret that decision.

@rebello95
Copy link
Collaborator Author

rebello95 commented Aug 15, 2024

Thanks for the review @eseay! Honestly I noticed because of the failing test and then looked into it more. Surprising that there's an actual behavior change between the existing function and the suggested replacement...

@rebello95 rebello95 merged commit 51d24b0 into main Aug 15, 2024
17 checks passed
@rebello95 rebello95 deleted the update-swiftlint branch August 15, 2024 13:00
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.

2 participants