Fix frozen string literal issue for ruby 3.4 #27
Conversation
chaadow
force-pushed
the
patch-1
branch
5 times, most recently
from
190c510
to
2509954
Compare
|
Also the CI should be enabled here I'll repush when it's done |
| @@ -12,5 +12,21 @@ permissions: | |||
| contents: read | |||
|
|
|||
| jobs: | |||
| Shared: | |||
| uses: fog/.github/.github/workflows/[email protected] | |||
There was a problem hiding this comment.
We'll want to drop this and use the shared one again I think once we are satisfied (though perhaps we'll want to update the shared one with RUBYOPTS in some way also).
There was a problem hiding this comment.
@geemus I just had the idea, but I can create a shared config on fog/.github for fog-aws fog-xml and fog-core.
and once all gems have migrated to this new shared config. we can completely remove the old config.
If you agree with this, please let me know and I'll go ahead and create a PR on fog/.github 👌
There was a problem hiding this comment.
We'll have to figure out some specifics, but yeah that sounds fine to me.
There was a problem hiding this comment.
Also or instead, we could leave this for now, but also add the shared workflow back (so that it just runs both). I didn't want to lose the shared workflow. Both is probably redundant, but it would be ok as an interim step.
|
Hi @geemus just like in the other PR, I believe you need to enable CI, thanks! |
|
Hmm. It looks like it should be enabled, but it's not picking this up. I'm going to try closing and reopening and see if that kicks it off. |
|
I think the error should be fixed if/when I release fog-core (since it will solve the base64 dependency issue). I'll try to do that tomorrow probably. Will also look at the other warnings and see what can be done (one will be me figuring out logging + excon, and I need to try and figure out what's up with the circular require, my brain isn't wanting to parse it just now). So I'll try to circle back soon. |
|
Ok, I released v2.5.0 of fog-core which had the other 3.4+ fixes and reran the tests here, which now also pass as I suspected. I think there might be some warnings remaining, but we can circle back to those later. |
@geemus Related to fog/fog-aws#711
The gem is tiny in size, so I was able to detect other frozen literal issues.
Went ahead and added a missing spec file ( didn't add full coverage of all the remaining classes, but it's better than nothing)
And I was able to test all cases where frozen string errors might arise.