search

vapor / apns

Helpful extensions and abstractions for using APNSwift
MIT License
115 stars 30 forks source link

Pass `Request.logger` as default logger parameter #38

Closed mxcl closed 3 years ago

mxcl commented 3 years ago

Here https://github.com/vapor/apns/blob/main/Sources/APNS/Request%2BAPNS.swift#L30

code28 commented 3 years ago

The whole logger configuration is a bit confusing, but if I see this correctly, this is (indirectly) already achieved by this line, isn't it? I was trying to explicitly remove the logging, but passing nil didn't work, because it took the request logger by default.

mxcl commented 3 years ago

K cool, thanks.

IMO this is bad design from APNSwift since it makes it impossible to not log, and violates my least surprise principle but that's an upstream problem.

code28 commented 3 years ago

K cool, thanks.

IMO this is bad design from APNSwift since it makes it impossible to not log, and violates my least surprise principle but that's an upstream problem.

I agree 100 %. It's on my list to think about it and open an PR to APNSwift. First I opened an issue to discuss. But if you want to go for it right now, it's also fine for me, because I probably won't make it until october.