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 support for bundler 2.2.25+ #446

Merged
merged 1 commit into from
Aug 23, 2021
Merged

Add support for bundler 2.2.25+ #446

merged 1 commit into from
Aug 23, 2021

Conversation

RyanBrushett
Copy link
Contributor

Motivation

Add support for Bundler 2.2.25 and up.
Fix #413

Implementation

  • Keep support for < 2.2.25 by leaving that codepath mostly unchanged.
  • Use the current version of the materialize method for bundler versions 2.2.25 and up.
  • I refactored a bit. If this makes it more confusing, let me know and I can rearrange some things. Also: naming variables is hard.

Tests

  • I added a 2.2.22 and 2.2.25 bundler versions to the test matrix, otherwise relying on the tests that were there.

@RyanBrushett RyanBrushett requested a review from a team August 19, 2021 18:30
@RyanBrushett
Copy link
Contributor Author

Sidenote: Do we need the actions to run on Push and PR or would just having it on Push be OK?

@RyanBrushett
Copy link
Contributor Author

Hmm those tests are passing locally. 👀ing.

@vinistock
Copy link
Member

bin/test uses bundle exec to execute tests. I don't think that is picking up the right bundler version. You'd need to change that CI step to be

bundle _${{ bundler_version }}_ exec ...

Also, you don't need quotes in the matrix versions.

@RyanBrushett
Copy link
Contributor Author

RyanBrushett commented Aug 19, 2021

Tidied up! Thanks Vini.

Those same tests are still failing, will keep digging! EDIT: I think a change in smart properties means the test needs a cheeky update. Curious why it isn't failing locally though...
I'm gonna remove the pull_request CI option and leave push. If someone's strongly opposed, I'll tack it back on.

lib/tapioca/gemfile.rb Outdated Show resolved Hide resolved
@RyanBrushett RyanBrushett requested review from paracycle and a team August 19, 2021 21:10
lib/tapioca/gemfile.rb Outdated Show resolved Hide resolved
spec/tapioca/cli/gem_spec.rb Outdated Show resolved Hide resolved
@RyanBrushett
Copy link
Contributor Author

RyanBrushett commented Aug 23, 2021

I'll fix the branch protection and ⛵ .

EDIT actually I'll ⛵ first so I don't block other people by setting branch protection that they can't possibly meet.

@RyanBrushett RyanBrushett merged commit a997565 into main Aug 23, 2021
@RyanBrushett RyanBrushett deleted the ryanb-support-bundler branch August 23, 2021 22:15
paracycle pushed a commit that referenced this pull request Aug 25, 2021
@shopify-shipit shopify-shipit bot temporarily deployed to production September 7, 2021 16:35 Inactive
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.

Bundler 2.2.25: `materialize': wrong number of arguments (given 2, expected 1) (ArgumentError)
4 participants