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

Fix customize_gemfiles on Ruby 3 #214

Conversation

yevhenii-ponomarenko
Copy link
Contributor

@yevhenii-ponomarenko yevhenii-ponomarenko commented Aug 16, 2023

Summary

#191 added an ability to customize headers in gemfiles generated by Appraisal, as well as creating the strings using single quotes. But there's a small bug with the implementation that prevents it from working on Ruby 3:

def customize_gemfiles(&_block)
  Customize.new(yield)
end

With changes in Ruby 3, the Hash yield'ed from the block needs to be converted into keyword arguments with **.
This PR resolves the issue and provides tests to validate this behaviour.

This addresses #213.

spec/appraisal/appraisal_file_spec.rb Outdated Show resolved Hide resolved
spec/appraisal/appraisal_file_spec.rb Show resolved Hide resolved
spec/appraisal/appraisal_file_spec.rb Outdated Show resolved Hide resolved
@joe-sharp
Copy link
Contributor

@nickcharlton you free to merge this?

@nickcharlton
Copy link
Member

Great! Thanks for contributing this, and sorry for the delay.

@joe-sharp, thanks for the ping, I appreciate it.

@luke-hill
Copy link

@nickcharlton - This looks more like the issue I found

@nickcharlton
Copy link
Member

Thanks! I'm keen to do a release with this, but I don't want to do so until I've gotten to the bottom of Bundler incompatibilities. In the short-term, you might wish to pin against the commit where this has been merged in.

@tisba
Copy link
Contributor

tisba commented Feb 16, 2024

@nickcharlton Do you suspect more issues? Or are we basically talking about #218?

@nickcharlton
Copy link
Member

We're talking about #218. There are a few deprecation warnings I've been looking into, but I don't think those will be a problem just yet.

@tisba
Copy link
Contributor

tisba commented Feb 16, 2024

I would suggest to switch to absolute paths for the tests when referencing local git repositories. I'm still not 100% sure I understand Bundler intentions (see comments on rubygems/rubygems#4475), but I'll try to spend some more time on it to understand this better.

If for appraisal this is just about its own specs, I'd just change them, tbh.

@nickcharlton
Copy link
Member

Yeah, just changing them seems like a good idea. It's always going to be awkward working through private APIs like we seem to be.

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.

5 participants