-
Notifications
You must be signed in to change notification settings - Fork 15
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
Use core minimum but allow 7.0/7.1/7.2 since core sets upper limit (breaking change) #40
Use core minimum but allow 7.0/7.1/7.2 since core sets upper limit (breaking change) #40
Conversation
970eb30
to
f7be3ef
Compare
@@ -7,7 +7,7 @@ Gem::Specification.new do |spec| | |||
spec.name = "activerecord-id_regions" | |||
spec.version = ActiveRecord::IdRegions::VERSION | |||
spec.authors = ["ManageIQ Developers"] | |||
|
|||
spec.metadata['rubygems_mfa_required'] = 'true' |
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.
the cops were complaining about this... everyone's using MFA right?
@@ -20,8 +20,8 @@ Gem::Specification.new do |spec| | |||
spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } | |||
spec.require_paths = ["lib"] | |||
|
|||
spec.add_dependency "activerecord", ">= 5.0", "<7.1" | |||
spec.add_dependency "activesupport", ">= 5.0", "<7.1" |
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.
this is the breaking change
This is ready for review |
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.
This looks great.
I'm good with breaking.
Can we s/3.2/3.3/g
?
f7be3ef
to
530c3db
Compare
Since this is a standalone gem, we should add all the versions, so I'd prefer to keep 3.2. Almost wondering if we should also keep 2.7, but that makes the rails excludes harder. |
I can put 2.7 back since rails 7 and 7.1 specify that as the minimum and this gem is a standalone gem, has fast tests and it's not a big deal to keep old rubies around. |
Drop end of life rails and add new versions. Ensure we're testing and supporting community supported versions. Drop: rails 6.0 rails 6.1 Add: ruby 3.2 ruby 3.3 rails 7.1 rails 7.2 Note: ruby 2.7, and 3.0 are not difficult to keep maintaining, this repo has fast tests and rails 7 is the minimum and it requires ruby 2.7+ so it's important to target the rubies supported by the dependency this repo tracks most closely.
530c3db
to
a62d109
Compare
Ok, added ruby 2.7 back since we support rails 7 as minimum and rails is the most important dependency and it supports 2.7 as a minimum. |
Drop end of life rails and add new versions.
Ensure we're testing and supporting community supported
versions.
Drop:
rails 6.0
rails 6.1
Add:
ruby 3.2
ruby 3.3
rails 7.1
rails 7.2
Note: ruby 2.7, and 3.0 are not difficult to keep maintaining, this repo has fast tests and
rails 7 is the minimum and it requires ruby 2.7+ so it's important to target the rubies supported
by the dependency this repo tracks most closely.