[rust gem] Major improvements for gem scaffolding (rebased)#8455
[rust gem] Major improvements for gem scaffolding (rebased)#8455hsbt merged 8 commits intoruby:masterfrom
Conversation
Yes, there was some overlapping with another PR, which was already merged, so that part is already there 👍. There's one test failure though that I'd say it's related since I've never seen it before, it may be intermittent given it only happened with MacOS + Ruby 3.1, but we should try to figure out the reason. |
|
It seems the failure was already noticed in the initial PR, see #7608 (comment). |
|
This is so helpful!! ❤️ |
What's Bundler's policy on supporting older rubies? Given that Ruby 3.1 is EOL in a month, can we get away with not fixing this? |
|
We'd be dropping support for Ruby 3.1 this year, so I'd be open to not fixing that error if it's just happening in Ruby 3.1. However, I think it may be affecting all rubies, because I believe @simi reproduced it locally with Ruby 3.3 as per #7608 (comment). |
|
Happy to test again if needed. |
|
@simi If you're able to confirm whether you still see the same issue locally, that'd be great! |
| # Uncomment to register a new dependency of your gem | ||
| # spec.add_dependency "example-gem", "~> 1.0" |
There was a problem hiding this comment.
Please restore these lines. They are not related Rust gem.
There was a problem hiding this comment.
You can maybe move it to an else block, since in the case of config[:ext] == 'rust', it should be unnecessary to explain how to add a dependency?
There was a problem hiding this comment.
It should be....but it's nice to do. :) I'll restore it.
| it "can call into Rust" do | ||
| result = <%= config[:constant_name] %>.hello("world") | ||
|
|
||
| expect(result).to be("Hello earth, from Rust!") |
There was a problem hiding this comment.
This needs eq as these will be different objects I think
Failure/Error: expect(result).to be("Hello earth, from Rust!")
expected #<String:1640> => "Hello earth, from Rust!"
got #<String:1648> => "Hello earth, from Rust!"
Compared using equal?, which compares object identity,
but expected and actual are not the same object. Use
`expect(actual).to eq(expected)` if you don't care about
object identity in this example.| expect(result).to be("Hello earth, from Rust!") | |
| expect(result).to eq("Hello earth, from Rust!") |
| expect(result).to be("Hello earth, from Rust!") | |
| expect(result).to eq("Hello world, from Rust!") |
| - name: Build gem | ||
| run: bundle exec rake build | ||
|
|
||
| - uses: actions/upload-artifact@v3 |
There was a problem hiding this comment.
|
Humble bump to @simi to verify that this PR resolves the previously observed issue. 🙏 |
| require_relative "<%= File.basename(config[:namespaced_path]) %>/version" | ||
| <%- if config[:ext] -%> | ||
| require_relative "<%= File.basename(config[:namespaced_path]) %>/<%= config[:underscored_name] %>" | ||
| # Attempt to load a versioned extension based on the Ruby version. |
There was a problem hiding this comment.
Can you remove these changes from template? That workaround is only for fat gem, not generally usage.
| it "includes rake-compiler, rb_sys gems and required_rubygems_version constraint" do | ||
| it "includes rake-compiler constraint" do | ||
| expect(bundled_app("#{gem_name}/Gemfile").read).to include('gem "rake-compiler"') | ||
| expect(bundled_app("#{gem_name}/#{gem_name}.gemspec").read).to include('spec.add_dependency "rb_sys"') |
e316bad to
c07c4e0
Compare
hsbt
left a comment
There was a problem hiding this comment.
I rebased with the current HEAD and squash some commits.
|
@ianks @gjtorikian I will merge this with the current changes. If you have extra changes, feel free to open another PR. Thanks! |
[rust gem] Major improvements for gem scaffolding (rebased) (cherry picked from commit 6b43b5e)
I heeded the call from @ianks in #7608 to rebase on top of
master.The one change I added was to bump the version of magnus to 0.7.
The one file diff not present is this one. I did not dig into the
git blame, but it looks like at some point Gemfile.tt removed this block.