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

Fix daemonization broken in #219 #224

Merged
merged 4 commits into from
Jun 8, 2016
Merged

Fix daemonization broken in #219 #224

merged 4 commits into from
Jun 8, 2016

Conversation

mariokostelac
Copy link
Contributor

No description provided.

it 'raises if Celluloid is already loaded' do
expect(cli).to receive(:celluloid_loaded?).and_return(true)
args = { daemon: true, logfile: '/dev/null' }
expect{ cli.send(:daemonize, args) }.to raise_error(RuntimeError)

Choose a reason for hiding this comment

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

Space missing to the left of {.

end

it 'raises if logfile is not set' do
expect { cli.send(:daemonize, { daemon: true }) }.to raise_error(ArgumentError)

Choose a reason for hiding this comment

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

Redundant curly braces around a hash parameter.


EnvironmentLoader.load(options)

load_celluloid
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we have to load celluloid later so we could attach logger properly.

@mariokostelac
Copy link
Contributor Author

@phstc you good with this PR? :)

@@ -64,21 +64,27 @@ def run(args)
private

def load_celluloid
raise "Celluloid cannot be required until here, or it will break Shoryuken's daemonization" if defined?(::Celluloid) && Shoryuken.options[:daemon]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@phstc it's just moved from here :)

@phstc
Copy link
Collaborator

phstc commented Jun 8, 2016

@mariokostelac looks good to me :shipit:

@mariokostelac mariokostelac merged commit 71fe6f9 into master Jun 8, 2016
@mariokostelac mariokostelac deleted the fix_daemonization branch June 8, 2016 15:02
@eugeneius
Copy link
Contributor

Sorry for breaking this 😩 and thanks for fixing it @mariokostelac !

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.

4 participants