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

add frozen_string_literal pragma to all files #711

Merged
merged 7 commits into from
Dec 17, 2020

Conversation

ritikesh
Copy link
Contributor

No description provided.

@ritikesh
Copy link
Contributor Author

@jnunemaker ping.

@jnunemaker
Copy link
Owner

@ritikesh Hi there! Thanks for the ping as this had slipped past me. Did you check each of the files to see if there were any modification of strings that would cause problems with this or just add the magic comment? I know the tests are passing, but I won't claim 100% test coverage. I just don't want to break anything for anyone.

@ritikesh
Copy link
Contributor Author

Hi @jnunemaker, guilty of over-depending on the tests. I'll have a look and give you a confirmation on the same. Thanks!

@ritikesh
Copy link
Contributor Author

Hey @jnunemaker, I looked at the code. Things look okay to me. I did modify some code however:

  1. removing redundant .freeze on constants
  2. reducing redundant allocations
  3. changed string literals

@jnunemaker jnunemaker merged commit f35528e into jnunemaker:master Dec 17, 2020
@ritikesh ritikesh deleted the frozen_string_literal branch December 17, 2020 18:31
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

2 participants