-
Notifications
You must be signed in to change notification settings - Fork 678
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
Clean up Appraisal and CI config; add Rails 7.2 and Ruby 3.3 #723
Changes from all commits
8031a1a
c4ca3d3
892baa9
821a3b6
575c29b
7b7a9de
053ca2e
831215b
af1efc5
83bab73
c7a002c
a28a6a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,11 +16,11 @@ Gem::Specification.new do |gem| | |
|
||
gem.required_ruby_version = ">= 2.3.0" | ||
|
||
gem.add_dependency "activerecord", ">= 5.2", "< 8.1" | ||
gem.add_dependency "activesupport", ">= 5.2", "< 8.1" | ||
gem.add_dependency "activerecord", ">= 5.2", "< 8.0" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nooo rip. that means when rails 8.0 is out i will have to fork this project again to be able to use rails main branch. i thought i had more time 😭 honestly i don't know why there is a restriction on the upper bound There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't really have a strong opinion here. I see this pattern of limiting the upper bound a lot, but is it really helping more than hurting? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My goal for this PR was to get CI passing and confirm Rails 7.2 compatibility. I removed the rails80 appraisal because it was not working. Without a passing test suite for 8.0, I didn't feel comfortable allowing 8.0 in the gemspec, and I don't have a real app running on Rails Rubygems itself frowns upon unbounded dependencies. I don't know how closely their recommendations are followed, but the Rubygems validation code emits a warning for it: For That said, ultimately if this is a very-lightly maintained gem, and the philosophy is "use it at your own risk", then an unbounded requirement makes sense. On the other hand, if there is a plan for actively maintaining and testing every release of Rails and Ruby versions going forward, including prereleases, then I think a more strict requirement would work. |
||
gem.add_dependency "activesupport", ">= 5.2", "< 8.0" | ||
|
||
gem.add_development_dependency "appraisal" | ||
gem.add_development_dependency "rails", ">= 5.2", "< 8.1" | ||
gem.add_development_dependency "rails", ">= 5.2", "< 8.0" | ||
gem.add_development_dependency "rspec-rails" | ||
gem.add_development_dependency "standard" | ||
gem.add_development_dependency "single_cov" | ||
|
This file was deleted.
This file was deleted.
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.
so in the changelog it says "Add rails 8 support" and now we remove the appraise
rails80
?