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

Require Ruby 2.6 #45

Merged
merged 3 commits into from
Jul 1, 2023
Merged

Require Ruby 2.6 #45

merged 3 commits into from
Jul 1, 2023

Conversation

ibrahima
Copy link
Contributor

I was trying to use this gem on Ruby 2.5 (sigh) and ran into an issue. It turns out that it calls then on a Hash, which was added in Ruby 2.6. I figured it would be helpful to document this explicitly, though hopefully there aren't a lot of people out there running 2.5.

https://til.hashrocket.com/posts/f4agttd8si-chaining-then-in-ruby-26

I was trying to use this gem on Ruby 2.5 (sigh) and ran into an issue. It turns out that it calls `then` on a Hash, which was added in Ruby 2.6. I figured it would be helpful to document this explicitly.

https://til.hashrocket.com/posts/f4agttd8si-chaining-then-in-ruby-26
@rubys
Copy link
Collaborator

rubys commented Jun 30, 2023

If .then is just an alias, and .then is only used twice that i can find, why not change both occurrences to .yield_self?

@ibrahima
Copy link
Contributor Author

Yeah that'd work too, I can make that change later. I didn't know if it was intentional or if there are any other dependencies on 2.6.

For Ruby 2.5 compatibility.
@ibrahima ibrahima changed the title Add required Ruby version to gemspec (2.6.0) Allow running on Ruby 2.5 Jun 30, 2023
@ibrahima
Copy link
Contributor Author

ibrahima commented Jun 30, 2023

@rubys done! I haven't tested this on Ruby 2.5 yet but it seems like it should work now...

Actually... maybe not. I now get this error, so maybe there's an issue with 2.5's version of ERB. Maybe something changed with how it handles partials? I'm confused since the syntax seems pretty normal to me... 🤷🏾.

	 5: from /usr/local/lib/ruby/2.5.0/erb.rb:876:in `result'
	 4: from /usr/local/lib/ruby/2.5.0/erb.rb:876:in `eval'
	 3: from /home/user/.bundle/ruby/2.5.0/dockerfile-rails-87c256966bdf/lib/generators/templates/Dockerfile.erb:39:in `template'
	 2: from /home/user/.bundle/ruby/2.5.0/dockerfile-rails-87c256966bdf/lib/generators/dockerfile_generator.rb:342:in `render'
	 1: from /usr/local/lib/ruby/2.5.0/erb.rb:874:in `result'
/usr/local/lib/ruby/2.5.0/erb.rb:872:in `block in result': no implicit conversion of Hash into Integer (TypeError)

The template line in question:

https://github.com/fly-apps/dockerfile-rails/blob/main/lib/generators/templates/Dockerfile.erb#L39

The part of ERB that's raising the error though I don't really understand why: https://github.com/ruby/ruby/blob/b7f19dd8419aa10c8bc3dfb8181a2caafe0d81d9/lib/erb.rb#L874

Personally... I'm not very invested in making this work on 2.5, I am in the process of upgrading anyway, and I can run the generator with 2.6 even if I'm building a 2.5 Docker image at the moment. Do you think it makes sense to just update the gemspec to specify 2.6+ for now? Thanks! I do really like this gem overall.

@ibrahima ibrahima changed the title Allow running on Ruby 2.5 Require Ruby 2.6 Jun 30, 2023
@rubys
Copy link
Collaborator

rubys commented Jun 30, 2023

Fix the RuboCop failure and I'll merge the change.

@ibrahima
Copy link
Contributor Author

Thanks! I left change of the alias method in for now, hope that's okay. If I get some time to figure out why it's not working on 2.5 I may submit another PR and loosen the restriction, but for now this should probably help some confusion for people on old Ruby versions.

@rubys rubys merged commit 65c3f36 into fly-apps:main Jul 1, 2023
3 checks passed
@ibrahima ibrahima deleted the patch-1 branch July 6, 2023 17:37
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.

2 participants