-
Notifications
You must be signed in to change notification settings - Fork 221
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
Move Rack::Logger
from rack
to rack-contrib
.
#194
Conversation
0d43932
to
6ec0519
Compare
Rack::Logger
.Rack::Logger
from rack
to rack-contrib
.
How would this work?
It's std now but not for long |
I think, if you're moving this here anyways, does it make sense to require the user to pass in an object that adheres to the rack logger interface themselves? |
The current plan is for us to wait and see if anyone complains about the deprecation. If no one complains, we'll probably remove it with no replacement. As you've said, the current implementation leaves a lot to be desired. |
I'm not sure I understand the rationale for moving this to
Since I understand that (1) is the plan, I kinda feel like this middleware would get essentially zero usage. |
This is preceded by the following:
|
Ah, thanks for pointing that out. I'd somehow skipped over that when refreshing my cache of |
@@ -0,0 +1,23 @@ | |||
# frozen_string_literal: true | |||
|
|||
require 'logger' |
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.
As I think @Earlopain pointed out earlier, the general criteria for inclusion in rack-contrib
is "no new external deps", and with Logger
getting evicted with Ruby 3.5, that gets a bit messy. Are you up for writing a super-slim pseudo-logger that just dumps to rack.errors
?
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.
Yes, it's one idea, I started writing it but then just thought, why am I doing this?
For now, let's just close this PR. |
No description provided.