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

Why is SHUTDOWN_TIMEOUT a constant? #159

Merged
merged 1 commit into from Mar 16, 2013

Conversation

halorgium
Copy link
Contributor

Shouldn't this be configurable by user code instead of being constant (will get warning if redefined)?

@tarcieri
Copy link
Member

Yeah, definitely. Should be user-customizable. It's just nobody's asked yet ;)

knewter pushed a commit to knewter/celluloid that referenced this pull request Mar 8, 2013
Remove a constant per the request in celluloid#159.  Replace with a class method
which returns the same default value.
@knewter
Copy link
Contributor

knewter commented Mar 8, 2013

@tarcieri what else should go into this? Assuming there's some goal to make it customizable, but is it enough for someone to just redefine the return value of this method? It's at least better than a constant. I'm always wary of configurability with class methods because I never know where to put the state.

@tarcieri
Copy link
Member

tarcieri commented Mar 8, 2013

@knewter looking at your PR, it should probably be an attr_accessor with a defined default. I can add that after-the-fact now that you've got the ball rolling

@coveralls
Copy link

Coverage remained the same when pulling 7e3efef on halorgium:shutdown-timeout-accessor into 76d78b6 on celluloid:master.

View Details

tarcieri added a commit that referenced this pull request Mar 16, 2013
@tarcieri tarcieri merged commit 3410f33 into celluloid:master Mar 16, 2013
shadoi pushed a commit to shadoi/celluloid that referenced this pull request Apr 18, 2013
Remove a constant per the request in celluloid#159.  Replace with a class method
which returns the same default value.
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.

None yet

4 participants