Add lifecycle announcements setting #1083
Conversation
For users who are using Sinatra to fake out third party services in an automated test suite, the startup/shutdown messages add clutter to test output. However, users may want to disable logging without silencing lifecycle announcements. So, add a separate option, `:lifecycle_announcements`, which addresses the startup and shutdown messages only. The default value is set to true to preserve current behavior.
|
Reviewing the original issue again, I realize this won't solve the issue entirely, because only Sinatra's part of the output is removed—the server's startup messages are still printed. Getting at those will probably be handler specific, though, so probably tricky. |
|
@sinatra/team-sinatra how should we proceed with this? |
|
The feature sounds good to me, but i'm not sold on the setting name. Perhaps we could think about a new name? Another option is to swallow stdout in this block in a way that would work for any handler (tests should help). Also, since we're bumping major we could change behavior to support |
|
Wouldn't it be simpler to either make a function logging_enabled? for that load of conditions before all the puts calls, or a log(String) method so you don't need to remember all of the conditions you need to check before calling puts. |
As far as I can tell, it's only 2 places. Or am I missing something? |
|
It is, but that doesn't mean it won't be used in other places in the future. |
| @@ -1416,7 +1416,11 @@ def quit! | |||
| return unless running? | |||
| # Use Thin's hard #stop! if available, otherwise just #stop. | |||
| running_server.respond_to?(:stop!) ? running_server.stop! : running_server.stop | |||
| $stderr.puts "== Sinatra has ended his set (crowd applauds)" unless handler_name =~/cgi/i | |||
|
|
|||
| unless lifecycle_announcements == false || handler_name =~/cgi/i | |||
There was a problem hiding this comment.
What about to use positive condition?
if lifecycle_announcements || handle_name !~ /cgi/g
Since that condition is used twice, what about to wrap it into some method?
There was a problem hiding this comment.
I think the right positive condition is if lifecycle_announcements && handle_name !~ /cgi/g ?
|
Hmm, the concept of this change seems good to me. Can you rebase & fix an issue simi pointed out? |
|
It looks like there is already a setting to disable these messages: https://github.com/sinatra/sinatra#available-settings (quiet option) |
Resolves #1077, #1551 by adding a separate setting from
:loggingfor folks who want to disable the startup/shutdown lifecycle announcements.