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

Committed improvements #35

Merged
merged 12 commits into from
Apr 10, 2018
2 changes: 1 addition & 1 deletion lib/guard/puma.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def start
end

def reload
return if Time.now - @last_restarted < options[:restart_timeout]
return if (Time.now - @last_restarted) < options[:restart_timeout]
@last_restarted = Time.now
Compat::UI.info "Restarting Puma..."
if options[:notifications].include?(:restarting)
Expand Down
42 changes: 31 additions & 11 deletions lib/guard/puma/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,26 @@ def initialize(options)
@options = options

puma_options = {
(pumactl ? '--config-file' : '--config') => options[:config],
'--control-token' => @control_token,
(pumactl ? '--control-url' : '--control') => "tcp://#{@control_url}"
puma_options_key(:config) => options[:config],
puma_options_key(:control_token) => @control_token,
puma_options_key(:control_url) => "tcp://#{@control_url}"
}
if options[:config]
puma_options['--config'] = options[:config]
else
puma_options['--port'] = options[:port]
end
%i[bind threads environment].each do |opt|
next unless options[opt]
if pumactl
next Compat::UI.warning(
"`#{opt}` option is not compatible with `pumactl` option"
)
%i[bind threads environment]
.select { |opt| options[opt] }
.each do |opt|
if pumactl
Compat::UI.warning(
"`#{opt}` option is not compatible with `pumactl` option"
)
else
puma_options["--#{opt}"] = options[opt]
end
end
puma_options["--#{opt}"] = options[opt]
end
puma_options = puma_options.to_a.flatten
puma_options << '--quiet' if @quiet
@cmd_opts = puma_options.join ' '
Expand Down Expand Up @@ -69,6 +71,24 @@ def sleep_time

private

PUMA_OPTIONS_KEYS_BY_PUMACTL = {
true => {
config: '--config-file',
control_url: '--control-url'
}.freeze,
Copy link
Owner

Choose a reason for hiding this comment

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

These inner hashes don't also need to be frozen. The outer hash being frozen freezes them. ❄️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even Strings in frozen Hashes not frozen, as you can see in this example. So, we maybe should freeze '--config-file' and other strings.

Copy link
Owner

Choose a reason for hiding this comment

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

Weird, I must have mistyped something in a previous IRB session.

I don't think it's really necessay to deep freeze the whole thing anyway, at least not right now (given no real issues on it)

false => {
config: '--config',
control_url: '--control'
}.freeze
}.freeze

private_constant :PUMA_OPTIONS_KEYS_BY_PUMACTL

def puma_options_key(key)
keys = PUMA_OPTIONS_KEYS_BY_PUMACTL[@pumactl]
keys.fetch(key) { |k| "--#{k.to_s.tr('_', '-')}" }
end

def run_puma_command!(cmd)
if pumactl
Kernel.system build_command(cmd)
Expand Down