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

Don't freeze the options hash #446

Closed
wants to merge 1 commit into from
Closed

Don't freeze the options hash #446

wants to merge 1 commit into from

Conversation

mmay
Copy link

@mmay mmay commented Oct 14, 2014

I couldn't figure out why freezing the options hash was a good idea. Any reason why the options hash is frozen?

In my case, I want to override an option after it has already been set and instead of using an instance variable I would prefer to just override the options hash.

@sferik
Copy link
Contributor

sferik commented Jan 20, 2015

It appears that the call to freeze was added in f7d9cc6.

@josevalim Do you remember why this was necessary?

@josevalim
Copy link
Contributor

It was frozen before here: f7d9cc6#diff-0065e6d4e143036f62fc86861719ddc1L117

It any case, it is probably due to safety. @mmay I think that instead of overriding it after initializing it, you should provide a class method that will build it with the proper option set in the first place?

@mmay
Copy link
Author

mmay commented Feb 2, 2015

Thanks for taking a look @sferik and @josevalim!

Using a class method to build the options hash is a better way to do what I wanted anyway.

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

3 participants