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

Remove support for old Bundler API in materialize_deps #1448

Merged
merged 3 commits into from
Mar 21, 2023
Merged

Conversation

KaanOzkan
Copy link
Contributor

Motivation

This piece of code was brittle and causes an error in the latest bundler version.

Implementation

Instead of supporting different method arity's which could keep changing in the future we can remove support for the old Bundler API. v2.2.25 has been out since July 2021. Users with old versions of Bundler will not be able to use future Tapioca versions.

Tests

Locally tested using BUNDLER_VERSION=2.4.9 bundle exec tapioca gem --verify and bumped the version used in tapioca to trigger CI run with latest bundler.

@KaanOzkan KaanOzkan requested a review from a team as a code owner March 21, 2023 16:38
Copy link
Member

@paracycle paracycle left a comment

Choose a reason for hiding this comment

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

Thanks!

"#{spec.name} (#{spec.version})"
end,
T::Array[String],
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This T.cast is no longer necessary, is the removal preferred?

Copy link
Member

Choose a reason for hiding this comment

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

Not sure if it is still necessary, but it would be great if we could remove it.

lib/tapioca/gemfile.rb Outdated Show resolved Hide resolved
lib/tapioca/gemfile.rb Outdated Show resolved Hide resolved
@@ -596,6 +596,7 @@ class Secret; end
end

it "must not include `rbi` definitions into `tapioca` RBI" do
@project.bundle_install
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There were some flaky errors that made this test fail by processing ruby2_keywords so I added a bundle install call to ensure temporary project is in a good state before continuing.

@KaanOzkan KaanOzkan merged commit 291c87c into main Mar 21, 2023
@KaanOzkan KaanOzkan deleted the ko/bundler-api branch March 21, 2023 21:23
@shopify-shipit shopify-shipit bot temporarily deployed to production March 21, 2023 22:07 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants