Application builder #224
Application builder #224
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## 2.x.x #224 +/- ##
==========================================
- Coverage 80.05% 79.76% -0.30%
==========================================
Files 97 96 -1
Lines 5947 5855 -92
==========================================
- Hits 4761 4670 -91
+ Misses 1186 1185 -1
☔ View full report in Codecov by Sentry. |
| }] | ||
| } else { | ||
| self.additionalChannelHandlers = [] | ||
| public init( |
There was a problem hiding this comment.
Is there a need for users to be able to initialize an HBApplication.Context?
There was a problem hiding this comment.
It is used by the XCT framework. If we were requiring swift 5.9 I guess I could use package scope
| .package(url: "https://github.com/apple/swift-nio-extras.git", from: "1.7.0"), | ||
| .package(url: "https://github.com/apple/swift-nio-http2.git", from: "1.20.0"), | ||
| .package(url: "https://github.com/apple/swift-nio-ssl.git", from: "2.14.0"), | ||
| .package(url: "https://github.com/apple/swift-nio-transport-services.git", from: "1.9.0"), | ||
| .package(url: "https://github.com/apple/swift-nio-transport-services.git", from: "1.18.0"), |
There was a problem hiding this comment.
Any particular reason for the specific minor increase? The newest one is 1.19.0
There was a problem hiding this comment.
1.18.0 introduces NIOTSEventLoopGroup.singleton. Means I don't need to provide the ability to create a new EventLoopGroup (which involves managing its lifecycle) I just provide the option of .singleton instead.
|
|
||
| // MARK: Methods | ||
|
|
||
| __consuming public func build() -> HBApplication { |
There was a problem hiding this comment.
Do we expect __consuming to stably stick around in/after 5.9?
There was a problem hiding this comment.
The stdlib is full of functions labelled __consuming. I added it because I was thinking of making the builder a move only type, so once it had built the application it was consumed and unavailable
There was a problem hiding this comment.
That makes sense to me, and I really like it. I'm just worried that it might not be supported down the line.
Sources/Hummingbird/AsyncAwaitSupport/Request+Persist+async.swift
Outdated
Show resolved
Hide resolved
- Deleted commented out code - FileMiddleware doesn't need an application to be constructed - `HBApplicationBulder.buildAndTest` returns value returned by closure
| /// } | ||
| /// app.buildAndTest(.router) { client in | ||
| /// // does my app return "hello" in the body for this route | ||
| /// app.XCTExecute(uri: "/hello", method: .GET) { response in |
There was a problem hiding this comment.
Shouldn't this be client.XCTExecute?
Split HBApplication into
Splitting the application into building phase and running phase types means I can have an immutable and Sendable running application.
The context has been split out as it is not possible to create a server as a member of the application which requires the application to initialise it. It also gives us more control over what we pass around.
The other thing to note is this PR also disable the ability to extend the application through
HBExtensions