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

Adopt swift-format #2794

Merged
merged 21 commits into from
Jul 19, 2024
Merged

Adopt swift-format #2794

merged 21 commits into from
Jul 19, 2024

Conversation

FranzBusch
Copy link
Member

Motivation:

We have been discussing the adoption of a formatting tool in our repositories for a long time and it turns out the time has come now with the addition of our new GH action pipelines we also want to setup formatting for our repos.

Modifications:

This PR configures swift-format very similar to how swift-certificates and swift-openapi-generator configured there's with only minimal differences. The goal is to land on one swift-format config that we can then use in all repos. The differences to the existing configurations are as follows:

NoLabelsInCasePatterns=true OpenAPI: false; Certs: false
OmitExplicitReturns=true OpenAPI: true, Certs: false
OrderedImports=true OpenAPI: false, Certs: false
ReplaceForEachWithForLoop=true OpenAPI: true, Certs: false
UseEarlyExits=false OpenAPI: false, Certs: true

These options are also different but only for the reason that they would cause a lot of time to adopt. New projects should start with them and we should enable them in the fullness of time.
ValidateDocumentationComments
AllPublicDeclarationsHaveDocumentation

Result:

We have a formatter configured and check for it in our CI.

# Conflicts:
#	IntegrationTests/tests_04_performance/test_01_resources/shared.swift
#	IntegrationTests/tests_04_performance/test_01_resources/test_ping_pong_1000_reqs_1_conn.swift
#	Sources/NIOCore/AsyncSequences/NIOAsyncWriter.swift
#	Sources/NIOCore/AsyncSequences/NIOThrowingAsyncSequenceProducer.swift
#	Sources/NIOCore/Codec.swift
#	Sources/NIOCrashTester/OutputGrepper.swift
#	Sources/NIOHTTP1/HTTPEncoder.swift
#	Sources/NIOHTTP1/HTTPServerPipelineHandler.swift
#	Sources/NIOHTTP1/NIOHTTPClientUpgradeHandler.swift
#	Sources/NIOHTTP1Client/main.swift
#	Sources/NIOHTTP1Server/main.swift
#	Sources/NIOMulticastChat/main.swift
#	Sources/NIOPerformanceTester/UDPBenchmark.swift
#	Sources/NIOUDPEchoClient/main.swift
#	Sources/NIOWebSocket/NIOWebSocketFrameAggregator.swift
#	Sources/NIOWebSocket/WebSocketProtocolErrorHandler.swift
#	Tests/NIOCoreTests/AsyncSequences/NIOThrowingAsyncSequenceTests.swift
#	Tests/NIOHTTP1Tests/HTTPClientUpgradeTests.swift
#	Tests/NIOHTTP1Tests/HTTPServerClientTest.swift
#	Tests/NIOHTTP1Tests/HTTPServerPipelineHandlerTest.swift
#	Tests/NIOHTTP1Tests/HTTPServerProtocolErrorHandlerTest.swift
#	Tests/NIOPosixTests/AsyncChannelBootstrapTests.swift
#	Tests/NIOPosixTests/ChannelTests.swift
#	Tests/NIOPosixTests/CodecTest.swift
#	Tests/NIOPosixTests/DatagramChannelTests.swift
#	Tests/NIOPosixTests/SALChannelTests.swift
#	Tests/NIOPosixTests/SocketChannelTest.swift
#	Tests/NIOPosixTests/StreamChannelsTest.swift
#	Tests/NIOWebSocketTests/WebSocketClientEndToEndTests.swift
@FranzBusch FranzBusch added the needs-no-version-bump For PRs that when merged do not need a bump in version number. label Jul 19, 2024
@FranzBusch FranzBusch merged commit c9756e1 into main Jul 19, 2024
26 checks passed
@FranzBusch FranzBusch deleted the fb-formatting branch July 19, 2024 09:48
FranzBusch added a commit that referenced this pull request Jul 19, 2024
# Motivation

We just adopted `swift-format` in #2794. To avoid blame showing this format commit we can use a `.git-blame-ignore-revs` file.

# Modification

This PR adds a `.git-blame-ignore-revs` and puts the formatting commit into it.

# Result

Helps in blaming the right person ;)
FranzBusch added a commit that referenced this pull request Jul 19, 2024
# Motivation

We just adopted `swift-format` in #2794. To avoid blame showing this format commit we can use a `.git-blame-ignore-revs` file.

# Modification

This PR adds a `.git-blame-ignore-revs` and puts the formatting commit into it.

# Result

Helps in blaming the right person ;)
FranzBusch added a commit that referenced this pull request Jul 19, 2024
# Motivation

We just adopted `swift-format` in #2794. To avoid blame showing this format commit we can use a `.git-blame-ignore-revs` file.

# Modification

This PR adds a `.git-blame-ignore-revs` and puts the formatting commit into it.

# Result

Helps in blaming the right person ;)
@@ -36,7 +36,7 @@ extension ByteBuffer {
// this is not technically correct because we shouldn't just bind
// the memory to `UInt8` but it's not a real issue either and we
// need to work around https://bugs.swift.org/browse/SR-9604
Array<UInt8>(UnsafeRawBufferPointer(fastRebase: ptr[range]).bindMemory(to: UInt8.self))
[UInt8](UnsafeRawBufferPointer(fastRebase: ptr[range]).bindMemory(to: UInt8.self))
Copy link
Member

Choose a reason for hiding this comment

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

@FranzBusch this is against the Swift style guides

Copy link
Member Author

Choose a reason for hiding this comment

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

Which style guide?

@@ -21,8 +21,8 @@ public struct ByteBufferView: RandomAccessCollection, Sendable {
public typealias Index = Int
public typealias SubSequence = ByteBufferView

/* private but usableFromInline */ @usableFromInline var _buffer: ByteBuffer
Copy link
Member

Choose a reason for hiding this comment

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

@FranzBusch why are we losing all these comments

Copy link
Member Author

Choose a reason for hiding this comment

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

That was on me and I can bring those back. We have a formatting rule that disallows block comments. I removed some of those private but... comments since we are super inconsistent with them throughout the project and all the properties are _. We have been consistent with the _ but not so much with the private but... comment.

}

@inlinable
public func index(after i: Index) -> Index {
return i + 1
Copy link
Member

Choose a reason for hiding this comment

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

why are we losing the returns? That makes debugging so much harder because now you can't just add a print above :(

FranzBusch added a commit that referenced this pull request Jul 19, 2024
# Motivation

We just adopted `swift-format` in #2794. To avoid blame showing this format commit we can use a `.git-blame-ignore-revs` file.

# Modification

This PR adds a `.git-blame-ignore-revs` and puts the formatting commit into it.

# Result

Helps in blaming the right person ;)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-no-version-bump For PRs that when merged do not need a bump in version number.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants