Skip to content

Support Ruby 4 - #138

Open
The-Alchemist wants to merge 2 commits into
Shopify:mainfrom
The-Alchemist:ruby-4-compatibility
Open

The-Alchemist wants to merge 2 commits into
Shopify:mainfrom
The-Alchemist:ruby-4-compatibility

Conversation

@The-Alchemist

@The-Alchemist The-Alchemist commented Sep 3, 2026 •

Copy link
Copy Markdown

Summary of Changes

  • Declare tsort as a runtime dependency (it leaves the default gems in Ruby 4.1) and bump rake to ~> 13.2 so the default task can load without ostruct on Ruby 4.
  • Require Ruby >= 3.3.0, retarget CI to 3.3/3.4/4.0, and update the JSON writer spec for json 2.8+ empty-array pretty-print. (Ruby 3.2 is EOL)
  • Freeze read-only constants for current RuboCop; leave Pass::SUBCLASSES mutable because subclasses still register into it.

@The-Alchemist
The-Alchemist marked this pull request as ready for review September 3, 2026 17:24
@The-Alchemist
The-Alchemist marked this pull request as draft September 3, 2026 18:27
@The-Alchemist
The-Alchemist marked this pull request as ready for review September 3, 2026 18:42

@eregon eregon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thank you.
Not super found of the noise of .freeze on constants, but since that seems the shopify style probably no good enough reason to deviate from it here.

Comment thread seafoam.gemspec Outdated
Comment on lines +28 to +31
# rake < 13.2 requires ostruct, which is not a default gem in Ruby 4.
spec.add_development_dependency("rake", "~> 13.2")
spec.add_development_dependency("rspec", "~> 3.8")
spec.add_development_dependency("rubocop-shopify", ">= 2.9.0")
spec.add_development_dependency("rubocop-shopify", ">= 3.0")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd move these to the Gemfile, unless there is an advantage to keep them here?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

i don't think there's an advantage, but I'm not as familiar with the code base

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

done! see a952690

wdyt?

Comment thread .ruby-version Outdated
@@ -1 +1 @@
4.0.6

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this needed? If not could you remove this commit?
(maybe it was just to trigger CI again?)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

i wasn't sure whether to keep .ruby-version or not. i was confused, to be honest. i don't know what version I should pin it to, but I don't think I can say >4.

whatever you think is best

@eregon

eregon commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

CI needs an approval from @nirvdrum to run

@nirvdrum

nirvdrum commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

@The-Alchemist Thanks for the PR. Modifying 126 files to support Ruby 4 seems wrong to me. Did you accidentally commit changes in this PR? If you have other changes you'd like to make, please pull those out into a separate PR so they can be evaluated separately.

@The-Alchemist

Copy link
Copy Markdown
Author

@nirvdrum you're right, my bad! i should have put that stuff on a separate branch. will update

tsort is leaving the default gems in 4.1, rake 13.0.6 cannot load
without ostruct on Ruby 4, and json 2.8+ changed empty-array pretty
print. Drop EOL Rubies and freeze read-only constants for current RuboCop.

Signed-off-by: The-Alchemist <kap4020@gmail.com>
Keep tsort as a runtime dependency so Ruby 4.1 installs still get it;
rake, rspec, and rubocop-shopify are only needed in this repo.

Signed-off-by: The-Alchemist <kap4020@gmail.com>

This branch has not been deployed

No deployments
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.

3 participants