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

Enable frozen_string_literal feature. #1076

Merged
merged 1 commit into from
Feb 2, 2016

Conversation

marshall-lee
Copy link
Contributor

Hello!

'literal'.freeze' fever not affected Sinatra and it's cool because now we have a better option: frozen_string_literal pragma introduced in Ruby 2.3.0!

@marshall-lee marshall-lee force-pushed the frozen_string_literal branch from 47aa552 to 6ddc73c Compare January 22, 2016 13:22
@kgrz
Copy link
Member

kgrz commented Jan 22, 2016

The pedantic side of me tells me this PR might be better suited for the 2.0 branch. But I can't find any reason not to add the literal for 1.4.x, going by the fact that 1.4.x can be used with Ruby 2.3 if the user so wishes. Open to opinions.

@marshall-lee
Copy link
Contributor Author

@kgrz thank you for your response!

this PR might be better suited for the 2.0 branch

What can I do for this?

@zzak
Copy link
Member

zzak commented Jan 24, 2016

I'm against merging this for 1.4.x because it may introduce subtle bugs that will be hard to trackdown / fix.

@zzak
Copy link
Member

zzak commented Jan 31, 2016

@marshall-lee Could you possibly rebase? Thanks

@marshall-lee
Copy link
Contributor Author

@zzak sure!

@marshall-lee marshall-lee force-pushed the frozen_string_literal branch from 6ddc73c to 56a6141 Compare February 1, 2016 21:39
@marshall-lee
Copy link
Contributor Author

Rebased build seems to be okay

zzak pushed a commit that referenced this pull request Feb 2, 2016
Enable frozen_string_literal feature.
@zzak zzak merged commit 1666dad into sinatra:master Feb 2, 2016
@zzak
Copy link
Member

zzak commented Feb 2, 2016

@marshall-lee Thanks!

@zzak zzak added this to the 2.0.0 milestone Aug 21, 2016
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