Skip to content
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

Allow for disabling the shutdown message #292

Merged
merged 2 commits into from
Jan 18, 2017
Merged

Allow for disabling the shutdown message #292

merged 2 commits into from
Jan 18, 2017

Conversation

twisterghost
Copy link
Contributor

By default, it is still on, but config.shutdown_message can toggle it off:

# Disable the shutdown message
Kemal.config.shutdown_message false

By default, it is still on, but config.shutdown_message can toggle it off
@@ -42,6 +43,14 @@ module Kemal
@logger = logger
end

def shutdown_message(status : Bool)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shutdown_message=

@shutdown_message = status
end

def shutdown_message?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be implemented as a getter method like

getter :shutdown_message

@@ -17,6 +17,9 @@ module Kemal
property host_binding, ssl, port, env, public_folder, logging, running,
always_rescue, serve_static : (Bool | Hash(String, Bool)), server, extra_options

getter :shutdown_message
setter :shutdown_message

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do not use property here? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Still learning crystal, should have looked up that line above 💃

@twisterghost
Copy link
Contributor Author

This also fixes the other half of #290

@twisterghost
Copy link
Contributor Author

Is there anything else you'd like me to update or is there a chance for this to be merged?

@sdogruyol
Copy link
Member

Hey thanks for the PR. I wasn't sure about this till now 👍

Could you please also send a PR documentation for this at https://github.com/kemalcr/kemalcr.com

@sdogruyol sdogruyol merged commit c08bf71 into kemalcr:master Jan 18, 2017
@twisterghost
Copy link
Contributor Author

Will do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants