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

Projects
None yet
3 participants
@marshall-lee
Contributor

marshall-lee commented Jan 22, 2016

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!

@kgrz

This comment has been minimized.

Show comment
Hide comment
@kgrz

kgrz Jan 22, 2016

Member

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.

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

This comment has been minimized.

Show comment
Hide comment
@marshall-lee

marshall-lee Jan 22, 2016

Contributor

@kgrz thank you for your response!

this PR might be better suited for the 2.0 branch

What can I do for this?

Contributor

marshall-lee commented Jan 22, 2016

@kgrz thank you for your response!

this PR might be better suited for the 2.0 branch

What can I do for this?

@zzak

This comment has been minimized.

Show comment
Hide comment
@zzak

zzak Jan 24, 2016

Member

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

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

This comment has been minimized.

Show comment
Hide comment
@zzak

zzak Jan 31, 2016

Member

@marshall-lee Could you possibly rebase? Thanks

Member

zzak commented Jan 31, 2016

@marshall-lee Could you possibly rebase? Thanks

@marshall-lee

This comment has been minimized.

Show comment
Hide comment
@marshall-lee

marshall-lee Feb 1, 2016

Contributor

@zzak sure!

Contributor

marshall-lee commented Feb 1, 2016

@zzak sure!

@marshall-lee

This comment has been minimized.

Show comment
Hide comment
@marshall-lee

marshall-lee Feb 1, 2016

Contributor

Rebased build seems to be okay

Contributor

marshall-lee commented Feb 1, 2016

Rebased build seems to be okay

zzak added a commit that referenced this pull request Feb 2, 2016

@zzak zzak merged commit 1666dad into sinatra:master Feb 2, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@zzak

This comment has been minimized.

Show comment
Hide comment
@zzak

zzak Feb 2, 2016

Member

@marshall-lee Thanks!

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