This repository has been archived by the owner. It is now read-only.

Don't require "posix/spawn" every time PosixRunner#available? is called #80

Merged
merged 1 commit into from Feb 28, 2015

Conversation

Projects
None yet
3 participants
@radarek
Contributor

radarek commented Feb 23, 2015

Fixes #79 "Optimization proposal for Cocaine::CommandLine::PosixRunner.available?".

true
rescue LoadError
false
return @available unless @available.nil?

This comment has been minimized.

@jyurek

jyurek Feb 23, 2015

Contributor

You don't need this line. The ||= will take care of this for you.

@jyurek

jyurek Feb 23, 2015

Contributor

You don't need this line. The ||= will take care of this for you.

This comment has been minimized.

@radarek

radarek Feb 23, 2015

Contributor

It won't because it will reassign for false value.

@radarek

radarek Feb 23, 2015

Contributor

It won't because it will reassign for false value.

This comment has been minimized.

@jyurek

jyurek Feb 23, 2015

Contributor

You're right. Good call.

@jyurek

jyurek Feb 23, 2015

Contributor

You're right. Good call.

@@ -42,6 +41,14 @@ def waitpid(pid)
Process.waitpid(pid)
end
def self.posix_spawn_gem_available?
require 'posix/spawn'

This comment has been minimized.

@houndci-bot

houndci-bot Feb 23, 2015

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@houndci-bot

houndci-bot Feb 23, 2015

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@radarek

This comment has been minimized.

Show comment
Hide comment
@radarek

radarek Feb 28, 2015

Contributor

@jyurek Is this pull ok for you or should I change something?

Contributor

radarek commented Feb 28, 2015

@jyurek Is this pull ok for you or should I change something?

@jyurek jyurek merged commit 80b0b8f into thoughtbot:master Feb 28, 2015

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
hound Hound has reviewed the changes.
@jyurek

This comment has been minimized.

Show comment
Hide comment
@jyurek

jyurek Feb 28, 2015

Contributor

No, it's good. Sorry about the delay. Thanks a lot!

Contributor

jyurek commented Feb 28, 2015

No, it's good. Sorry about the delay. Thanks a lot!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.