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

provide an after_connect configuration option to run a Proc immadiately ... #46

Merged
merged 1 commit into from
Sep 17, 2013

Conversation

mrbrdo
Copy link

@mrbrdo mrbrdo commented Sep 17, 2013

...after connection is made

JonathanTron added a commit that referenced this pull request Sep 17, 2013
provide an after_connect configuration option to run a Proc immadiately ...
@JonathanTron JonathanTron merged commit 0ffdd50 into TalentBox:master Sep 17, 2013
::Sequel.connect config['url'], config
else
::Sequel.connect config
end

callback = app.config.sequel.after_connect

Choose a reason for hiding this comment

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

Why not just use configuration instead of the app.config mess?

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, it should simplify the code. Thanks for the code review!

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I forgot I've changed how the Configuration is structured after having merged this, there is no such thing anymore in the code.

Thanks anyway for reviewing it.

Choose a reason for hiding this comment

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

You're welcome; just taking a peek at what changed before upgrading my apps :)

And sorry, my bad, if I'd have checked the current code I'd have noticed you fixed it already.

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