Support Rails 7.1 and 7.2 - #45
Merged
Merged
Conversation
The gemspec required Rails 8.0 from the first release, so applications on the two prior Rails lines could not add the gem at all. Measuring the suite against those lines shows the floor was higher than the code needs. On Rails 7.1 and 7.2 the suite fails one test, and that test asserts a Rails 8 error class rather than a behaviour: Rails 8 maps an exhausted SQLite busy handler to StatementTimeout, and 7.1 and 7.2 leave it as the StatementInvalid that timeout subclasses. The message the caller relies on is identical, so the assertion moves to the parent class and keeps the message match. The bundled migrations declared ActiveRecord::Migration[8.0], which raises Unknown migration version on 7.x. They now declare [7.1]. Under Rails 8 the compatibility layer between the two only changes remove_foreign_key, and no Solid Objects migration calls it, so an install built at [7.1] gets the same schema as one built at [8.0]. Rails 7.0 stays out of range. Its SQLite adapter requires sqlite3 ~> 1.4, and the busy-handler control this gem depends on arrived in sqlite3 2.x, so the two cannot be loaded together. The compatibility matrix now runs Ruby 3.3, 3.4, and 4.0 against Rails 7.1, 7.2, 8.0, and 8.1. That job runs SQLite only, so adapter behaviour on the older lines is unmeasured against PostgreSQL and MySQL, and the roadmap says so.
Greptile SummaryThe PR lowers the supported Rails floor from 8.0 to 7.1 and extends compatibility coverage accordingly.
Confidence Score: 5/5The PR appears safe to merge, with the dependency, migration, CI, test, and documentation changes consistently reflecting the Rails 7.1 floor. The requested Rails line is pinned consistently in compatibility jobs, the migration operations remain unchanged, and the broadened lock assertion preserves the observable exhausted-lock contract across supported Rails versions. Important Files Changed
Reviews (1): Last reviewed commit: "Support Rails 7.1 and 7.2" | Re-trigger Greptile |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds to the untagged
0.13.3release.The gemspec required Rails 8.0 from the first release commit, so applications on
the two prior Rails lines could not add the gem. Measuring the suite against
those lines shows the floor was higher than the code needs.
What the measurement showed
Full suite, SQLite, with the gemspec floor relaxed and no other change:
sqlite3 ~> 1.4conflictThe single failure was the same one in all three passing runs, and it asserts a
Rails 8 error class rather than a behaviour:
Rails 8 maps an exhausted SQLite busy handler to
StatementTimeout. Rails 7.1and 7.2 leave it as the
StatementInvalidthatStatementTimeoutsubclasses.The library's own retry predicate walks the cause chain for
BusyException|BusyError, so it never depended on the Rails 8 class. Theassertion moves to the parent class and keeps the
/database is locked/match,which is what the caller actually relies on.
Rails 7.0 is out of range
Its SQLite adapter requires
sqlite3 ~> 1.4:The busy-handler control this gem depends on (
busy_handler_timeout=) arrivedin
sqlite32.x, so the two cannot be loaded together. Supporting 7.0 wouldmean giving up SQLite, which the gem advertises.
Change
>= 8.0to>= 7.1foractioncable,actionpack,actionview,activerecord,activesupport, andrailties.rack >= 3.1is unchanged and still resolves on 7.1.ActiveRecord::Migration[7.1].[8.0]raisesUnknown migration versionon 7.x. Under Rails 8 the compatibility layerbetween
[7.1]and[8.0]only changesremove_foreign_key, and no SolidObjects migration calls it, so an install built at
[7.1]gets the sameschema as one built at
[8.0].7.2, 8.0, and 8.1.
docs/development.md,AGENTS.md,docs/roadmap.md, andCHANGELOG.mdrecord the new floor.Limitation recorded
The compatibility job runs SQLite only. The PostgreSQL and MySQL jobs run on the
newest Rails, so adapter behaviour on Rails 7.1 and 7.2 is unmeasured against
those servers.
docs/roadmap.mdstates this rather than implying full coverage.Validation
bundle exec rakeon Rails 8.1.3.1 / Ruby 4.0.5: 535 runs, 1768 assertions,0 failures, 0 errors, 15 skips. Standard Ruby, RuboCop, RBS, Steep, and Brakeman
all clean. The skip count matches
main.