Run 3.4 build with frozen string mutation detection #3104
Conversation
| if is_ruby_34_plus; then | ||
| export RUBYOPT="$RUBYOPT -W:deprecated" | ||
| export RUBYOPT="$RUBYOPT --enable=frozen-string-literal --debug=frozen-string-literal" | ||
| fi |
There was a problem hiding this comment.
I think this needs to be applied only to our specs
There was a problem hiding this comment.
It’s the purpose to apply to everything:
This is going to be a community effort 💪.
To help in this transition, you can enable warnings in CI and note which gems issue warnings. Then, report them to the gem maintainers.
There was a problem hiding this comment.
Our CI should not be failing because other gems raise warnings, in particular I think this is why the yard check is failing.
I'm basicalling proposing we do the equivalent of:
RUBYOPT="$RUBYOPT -W:deprecated --enable=frozen-string-literal --debug=frozen-string-literal" rspec ...
So that its only running RSpec that flags issues
There was a problem hiding this comment.
Well, the intention wasn’t to fail our 3.4 build job when any of the (including development) dependencies mistakenly introduces a string literal mutation. But rather have such a build job as optional. If we’d observe a red one, that would tell us to check if it’s coming from the PR changes or from dependencies.
But it wouldn’t prevent from merging if it’s not our code.
For that reason, I planned to add two 3.4 build jobs, one with this check, and one conventional, required one.
A justification for this is to help the ecosystem.
We can probably even add this build yo main only and skip for PRs.
There was a problem hiding this comment.
I want all checks on our repos to pass because my experience is that having a failing build, even just ruby-head, becomes to easy to ignore when there are actual failures "oh it always fails so its fine syndrome".
If you want to have something that looks for warnings in the wider ecosystem, I'd recommend setting something up with a cron action in your own repo away from rspec, we don't actually use a lot of the ecosystem especially given our pinning of gems to older versions
|
I can’t recall why I decided to put this PR off for now, most probably Ruby 3.4 failures I didn’t want to deal with until it’s almost released. |
|
I'm going to close this for now, at some point I would like us to ensure that all our code is compatible with these flags, but hopefully the monorepo will be done before Ruby 3.4 |
https://gist.github.com/fxn/bf4eed2505c76f4fca03ab48c43adc72