-
Notifications
You must be signed in to change notification settings - Fork 33
Add ruby 3.4 CI and gem base64 listed #124
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
Conversation
8642f4f to
8f0eb33
Compare
|
Hey @hmdros I was reviewing your PR and running some tests around it, I wonder if that might help us to have a smooth inclusion of Ruby 3.4. I have some questions in general.
I hope someone else shows up to answer those questions. If something in the test PR I opened tracks on you feel free to pick it up and close my PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #124 +/- ##
==========================================
- Coverage 98.41% 93.98% -4.44%
==========================================
Files 17 26 +9
Lines 315 515 +200
==========================================
+ Hits 310 484 +174
- Misses 5 31 +26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1147a2d to
c90921f
Compare
Thanks for your eyes here. I had previously CI failing with bundler -v 2.4.6 and thought I needed all that constraints to get a green CI. Didn't even notice that Ruby 2.6 was compatible with bundler -v 2.4.22. |
If you’re developing a gem, dependencies that are required for the gem to function (in production or when used by others) should go in the .gemspec file under spec.add_dependency. Dependencies only used during development or testing (e.g. RSpec, RuboCop, Pry) should go in the Gemfile or as add_development_dependency in the gemspec. So if your gem’s runtime code calls Base64 directly, it belongs in the gemspec. In Ruby 3.4+,
Yes, omitting the version is fine if your gem works with all released versions of base64. Ruby will automatically install the latest compatible version. You only need to pin a version if you rely on behavior that changed between releases or if a specific version caused regressions. I think it is fine in our case to let base64 get resolved during gem installation.
Before Ruby 3.4, base64 was part of the standard library — no gem installation required. From Ruby 3.4 onwards, it was extracted into a separate gem. That means: For Ruby <3.4, require 'base64' just loads the built-in version. For Ruby ≥3.4, Bundler will install the base64 gem listed in your gemspec. You don’t need to manually manage which version Ruby picks — it’s handled automatically by Bundler and RubyGems. You’d only specify a version if you want to enforce compatibility with a specific release of the gem (for example, base64 >= 0.1.1). Your gemspec dependency ensures that anyone using your gem on Ruby 3.4+ has base64 available — without breaking Ruby 3.3 or older. I don't see why we are including base64, because we never
Not sure what you mean by bundler constraints? I understand that for standard or stdlib gems like base64, it’s usually unnecessary — you can rely on your Gemfile.lock to lock versions for CI. |
skunk.gemspec
Outdated
| spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } | ||
| spec.require_paths = ["lib"] | ||
|
|
||
| spec.add_dependency "base64" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hmdros
Adding the constraint because copilot suggested it in my PR (as a good practice), and the latest version of base64 supports from Ruby 2.4 to 3.4, meaning it does not hurt having it. Once you add this I'll approve it, thanks!
| spec.add_dependency "base64" | |
| spec.add_dependency "base64", "~> 0.3.0" |
It was being called when running tests and called in |
a7c0626 to
cc48f1b
Compare
well, webmock should be able to do whatever vcr is doing; maybe we can remove the vcr dependency and use only webmock. let me know if you are going to give it a shot, if not I can approve and merge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CHANGELOG.mdthat links to this PR under the "main (unreleased)" heading.Description:
This PR adds
base64as a listed gem when using Ruby 3.4, since it's not a default gem but a bundled gem in Ruby 3.4.REF: https://docs.ruby-lang.org/en/3.4/NEWS_md.html
I will abide by the code of conduct.