abstract registration function into a protocol #75
Conversation
Sources/Lifecycle/Lifecycle.swift
Outdated
|
|
||
| public protocol LifecycleRegistrar { |
There was a problem hiding this comment.
not in love with this name, better ideas?
|
@fabianfett as discussed for the lambda use case |
Sources/Lifecycle/Lifecycle.swift
Outdated
| self.register(label: label, start: .none, shutdown: handler) | ||
| } | ||
| } | ||
|
|
||
| internal struct LifecycleTaskImpl: LifecycleTask { |
There was a problem hiding this comment.
Is it more common to name these LifecycleTask: LifecycleTaskProtocol? *Impl is kind of Java-y.
There was a problem hiding this comment.
@yim-lee LifecycleTask is the public "currency type", while LifecycleTaskImpl is internal. While there are a few examples where Swift stdlib uses the XxxProtocol convention (e.g. StringProtocol) its actually not all that common to use it for public "currency type". That said I too not in love with LifecycleTaskImpl but since its internal I let it slide. wdyt?
There was a problem hiding this comment.
It's not too bad if it's internal; true that making the actual public name good is more important 👍
|
@tomerd Since we originally intended this change for Lambda, I think we should first find a solution to this: swift-server/swift-aws-lambda-runtime#141 (comment) Or do you want this change no matter what Lambda does? |
I think this is a good change in any case so I would merge this |
motivation: allow libraries that want to expose the lifecycle so that downstream code could register additional items to do so regardless of the lifecycle backend changes: abstract registration function into a protocol and conform ComponentLifecycle and ServiceLifecycle to it
tomerd
force-pushed
the
abstract-register
branch
from
1ec779c to
68facea
Compare
|
renamed wdyt? |
Sources/Lifecycle/Lifecycle.swift
Outdated
| /// - parameters: | ||
| /// - tasks: array of `LifecycleTask`. | ||
| func register(_ tasks: [LifecycleTask]) { | ||
| extension ServiceLifecycle: LifecycleTasksConainer { |
There was a problem hiding this comment.
| extension ServiceLifecycle: LifecycleTasksConainer { | |
| extension ServiceLifecycle: LifecycleTaskContainer { |
There was a problem hiding this comment.
LifecycleTaskContainer sounds good :)
Sources/Lifecycle/Lifecycle.swift
Outdated
|
|
||
| func start(_ callback: @escaping (Error?) -> Void) { | ||
| self.start.run(callback) | ||
| extension ComponentLifecycle: LifecycleTasksConainer { |
There was a problem hiding this comment.
| extension ComponentLifecycle: LifecycleTasksConainer { | |
| extension ComponentLifecycle: LifecycleTaskContainer { |
Sources/Lifecycle/Lifecycle.swift
Outdated
|
|
||
| extension LifecycleTasksConainer { |
There was a problem hiding this comment.
| extension LifecycleTasksConainer { | |
| extension LifecycleTaskContainer { |
|
sounds good 👍 |
motivation: allow libraries that want to expose the lifecycle so that downstream code could register additional items to do so regardless of the lifecycle backend
changes: abstract registration function into a protocol and conform ComponentLifecycle and ServiceLifecycle to it