[rust gem] Major improvements for gem scaffolding (rebased) #8455
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. |
bundler/lib/bundler/templates/newgem/github/workflows/build-gems.yml.tt
Outdated
Show resolved
Hide resolved
|
@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. 🙏 |
6 participants
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.