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

fixed FrozenError caused by frozen-string-literal #390

Merged
merged 2 commits into from Mar 21, 2019
Merged

fixed FrozenError caused by frozen-string-literal #390

merged 2 commits into from Mar 21, 2019

Conversation

@taichi-ishitani
Copy link
Contributor

@taichi-ishitani taichi-ishitani commented Feb 28, 2019

Hi,

I got FrozenError when I used rubyzip gem with --enable-frozen-string-literal option.

FrozenError:
       can't modify frozen String, created at C:/Ruby/Ruby25-x64/lib/ruby/gems/2.5.0/gems/rubyzip-1.2.2/lib/zip/entry.rb:605

This PR is to fix the above error.

@taichi-ishitani taichi-ishitani changed the title fixed errors caused by frozen-string-literal fixed FrozenError caused by frozen-string-literal Feb 28, 2019
@coveralls
Copy link

@coveralls coveralls commented Feb 28, 2019

Coverage Status

Coverage increased (+0.4%) to 95.751% when pulling 0e6e626 on taichi-ishitani:master into d07b13a on rubyzip:master.

Copy link
Member

@jdleesmiller jdleesmiller left a comment

Thanks!

I'm helping Alex with maintenance now, and I'll get this merged. I think the problem with the CI is that it's testing on some versions of ruby that are now end of life, so I may remove the changes in 0e6e626, but they do help for now.

@jdleesmiller jdleesmiller merged commit 3219d8e into rubyzip:master Mar 21, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.4%) to 95.751%
Details
@jdleesmiller
Copy link
Member

@jdleesmiller jdleesmiller commented Mar 22, 2019

Linking this up: #374 also tried to address this. Neither PR is a superset of the other, so I'm not sure either one addressed the issue fully, but it is probably all still helping.

I think the best way forward is to get rubocop up to date and enforce the # frozen_string_literal: true magic comments in all the rubyzip files, so it's not dependent on end user flags.

@taichi-ishitani
Copy link
Contributor Author

@taichi-ishitani taichi-ishitani commented Mar 22, 2019

https://github.com/Invoca/magic_frozen_string_literal

I think this gem can help you to add the magic comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants