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: true comment to all files #290

Merged

Conversation

bentranter
Copy link
Member

@bentranter bentranter commented Oct 22, 2021

Attempts to address #289

While I thought the plan for Ruby 3 was to make frozen_string_literal: true the default, I guess that wasn't the case (see Matz's comment about it). This change adds all of the magic frozen_string_literal: true comments to the top of the file.

When run in IRB on main, this is what I see when trying to use a Ractor to print DropletKit's version:

 $ irb -I lib -r droplet_kit
irb(main):001:0> r = Ractor.new { puts DropletKit::VERSION }
<internal:ractor>:267: warning: Ractor is experimental, and the behavior may change in future versions of Ruby! Also there are many implementation issues.
#<Thread:0x00007fa85987e160 run> terminated with exception (report_on_exception is true):
(irb):1:in `block in <main>': can not access non-shareable objects in constant DropletKit::VERSION by non-main Ractor. (Ractor::IsolationError)
=> #<Ractor:#2 (irb):1 terminated>

which seems to be the error in the linked issue.

This is what I see on this branch:

$ irb -I lib -r droplet_kit
irb(main):001:0> r = Ractor.new { puts DropletKit::VERSION }
<internal:ractor>:267: warning: Ractor is experimental, and the behavior may change in future versions of Ruby! Also there are many implementation issues.
3.16.0
=> #<Ractor:#2 (irb):1 terminated>

Suggesting that it resolves the issue.

I also fixed the whitespace + formatting of a few files. It probably wasn't necessary to add the comment to all of the tests, but I did this with the magic_frozen_string_literal gem which did that by default, so I figured I'd leave it.

@bentranter bentranter requested a review from a team October 22, 2021 12:51
@andrewsomething
Copy link
Member

I thought the plan for Ruby 3 was to make frozen_string_literal: true the default

This comment it's self is three years old now, so not sure of the current state of affairs, but looks like matz decided not to do this.

So I officially abandon making frozen-string-literals default (for Ruby3).

https://bugs.ruby-lang.org/issues/11473#note-53

Copy link
Member

@andrewsomething andrewsomething left a comment

Choose a reason for hiding this comment

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

👍 Makes sense

@bentranter bentranter merged commit 2a7dede into digitalocean:main Oct 22, 2021
@bentranter bentranter deleted the add-frozen-string-literal-true branch January 26, 2022 23:04
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.

2 participants