-
Notifications
You must be signed in to change notification settings - Fork 225
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
ISSUE-462: Add rexml dependency for Ruby 3.0.0+ #467
ISSUE-462: Add rexml dependency for Ruby 3.0.0+ #467
Conversation
CHANGELOG.md
Outdated
@@ -2,6 +2,7 @@ | |||
|
|||
* [CHANGE] Fix test warning related to `cucumber_opts` declaration (by [@faisal][]) | |||
* [BUGFIX] Stop using long-deprecated MiniTest module name, removed in 5.19.0 (by [@faisal][]) | |||
* [CHANGE] Add rexml dependency for Ruby 3.0.0+ (by [@fbuys][]) |
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.
Did you add yourself at the bottom of the file?
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.
Checks failing because https://docs.rubocop.org/rubocop/cops_gemspec.html#gemspecrubyversionglobalsusage -> https://rubystyle.guide/#no-ruby-version-in-the-gemspec
Is checking the Ruby version worth it? I believe Bundler should use the installation-provided rexml if we don't care about its version. If we do care about the rexml version then we should care about it regardless of Ruby version.
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.
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.
LGTM, just add you name to the bottom of changelog and we can get this merged.
Thanks!
9ef5095
to
6ce85e2
Compare
We add rexml as a runtime dependency for environments that make use of Ruby 3 and up See: https://stackoverflow.com/a/65480744 Issue: whitesmith#462
6ce85e2
to
b4595cc
Compare
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.
LGTM. I ran the tests against Ruby 2.7, 3.0, 3.1, and 3.2, and it tested out cleanly for all.
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.
@fbuys, thank you!
@nunosilva800 can we merge? |
We add rexml as a runtime dependency for environments that make use of Ruby 3 and up.
See: https://stackoverflow.com/a/65480744
Issue: #462
rexml does not add any additional dependencies: https://rubygems.org/gems/rexml.
I did test that this works with Ruby 3.2.2 and Ruby 2.7.8.
Check list: