Skip to content

fixed FrozenError caused by frozen-string-literal #390

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.

Sign up for GitHub

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

Jump to bottom
Merged
jdleesmiller merged 2 commits into from
Mar 21, 2019
Merged

fixed FrozenError caused by frozen-string-literal #390

jdleesmiller 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 commented Feb 28, 2019 •
edited
Loading

Coverage Status

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

jdleesmiller
jdleesmiller approved these changes Mar 21, 2019
View reviewed changes
Copy link
Member

@jdleesmiller jdleesmiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
@jdleesmiller jdleesmiller mentioned this pull request Mar 21, 2019
Merged
@jdleesmiller
Copy link
Member

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 commented Mar 22, 2019

https://github.com/Invoca/magic_frozen_string_literal

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

@jdleesmiller jdleesmiller mentioned this pull request Feb 8, 2020
Merged
This was referenced Mar 11, 2021
Open
Open
Open
Open
Open
Open
Open
Open
Closed
Merged
Merged
Closed
Open
Open
Open
Open
Open
Open
Open
Open
This was referenced Mar 17, 2021
Merged
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jdleesmiller jdleesmiller jdleesmiller approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@taichi-ishitani @coveralls @jdleesmiller