-
Notifications
You must be signed in to change notification settings - Fork 134
Make Task.logger accessible to delegate implementations outside of Package #587
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
Make Task.logger accessible to delegate implementations outside of Package #587
Conversation
|
Can one of the admins verify this patch? |
6 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
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.
@fabianfett Would love to get your opinion on this one!
| public final class Task<Response> { | ||
| /// The `EventLoop` the delegate will be executed on. | ||
| public let eventLoop: EventLoop | ||
| public let logger: Logger // We are okay to store the logger here because a Task is for only one request. |
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.
Can you please add a simple doc comment to this property.
e277c8d to
1e901b9
Compare
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.
LGTM. However we should habe @glbrntt way in as well.
|
@swift-server-bot add to allowlist |
|
@swift-server-bot test this please |
…ckage Motivation: This change was proposed in issue [swift-server#389](swift-server#389). Users writing their own `HttpClientResponseDelegate` implementation might want to emit log messages to the `task`'s `logger`. Modifications: Changed the access level of `Task`'s `logger` property from `internal` to `public`. Result: Users can access the `logger` property of a `Task` outside of the `async-http-client`.
c15b42a to
72a12ec
Compare
|
@swift-server-bot test this please |
|
@felixschlegel thanks a lot for the patch! sorry for the delay. |
No problem 😄 Thanks for merging! |
Motivation:
This change was proposed in issue #389.
Users writing their own
HttpClientResponseDelegateimplementationmight want to emit log messages to the
task'slogger.Modifications:
Changed the access level of
Task'sloggerproperty frominternalto
public.Result:
Users can access the
loggerproperty of aTaskoutside of theasync-http-clientPackage.