Rails 8.1 - #112
Open
skunkworker wants to merge 9 commits into
Open
Rails 8.1#112skunkworker wants to merge 9 commits into
skunkworker wants to merge 9 commits into
Conversation
Pin activemodel/activesupport to ~> 8.0.0, following the branch convention of tracking a single Rails version instead of using Appraisal to span multiple versions. - Bump gem version to 8.0.0 (shadows Rails 8.0) - Require Ruby 3.2 (to match Rails 8.0) - Replace deprecated public_instance_methods.include? stub with public_method_defined? in association specs - Simplify CI to a single-version test matrix (no Appraisal) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pin activemodel/activesupport to ~> 8.1.0, following the branch convention of tracking a single Rails version instead of using Appraisal to span multiple versions. - Bump gem version to 8.1.0 (shadows Rails 8.1) - Require Ruby 3.2 (to match Rails 8.1) - Replace deprecated public_instance_methods.include? stub with public_method_defined? in association specs - Simplify CI to a single-version test matrix (no Appraisal) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop the class-level `@attribute_names ||=` memoization; it had no invalidation hook, so attributes declared after the first call went missing. Delegate to ActiveModel's already-invalidated `attribute_types` (matching upstream ActiveModel's own implementation). Add specs guarding the :value type registration and the protobuf serializer's :value fallback, plus negative-path coverage ([]= on unknown attribute, empty-collection query, readonly update_attribute). Harden the shared-mutable-default spec to use a throwaway class so it can't pollute shared fixture state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@liveh2o could we get a RC release of this? |
Splitting changes_applied into a #save snapshot and a conditional clear in #remote left #save's comment describing a mechanism it no longer has, and #instantiate undocumented while every sibling override was not. Co-Authored-By: Claude <noreply@anthropic.com>
An audit of every comment in lib/ found roughly 30 that no longer described the code. Three causes: prose copied from ActiveRecord that was never true here (integration.rb's cache_versioning claims are the inverse of what cache_key does, and its find examples raise), comments naming the wrong method, and comments that rotted when the code moved on. Two of the findings were not documentation problems. #attribute_for_inspect took only string names while its #[] and #[]= siblings coerce, so a symbol silently reported "nil" -- it now delegates to #[]. And .attr_publishable lost its only consumers when publication and the JSON serializer were removed in 3.0.0, leaving an inert macro; it is gone. Co-Authored-By: Claude <noreply@anthropic.com>
The association reader resolved the associated class before checking its own cache, so #classify ran the uncached inflector on every read -- 8us and 9 allocations to return a value already in an ivar. The cache check moves first. One consequence worth naming: validate_scoped_attributes now runs on the first read per instance rather than every read. build_from_rpc overwrites every slot it dups, so deep_dup there was pure waste on the per-record deserialization path; a shallow dup is enough. The dup in merge_attributes_from_rpc stays -- the mutation tracker holds the attribute set by reference, so replacing it is what makes #previous_changes work. #delete and #destroy had drifted into byte-identical bodies, which this branch made worse by adding the same errors.clear line to each; they now share a private remote_delete(endpoint). The adapter spec's hand-rolled failing client is replaced by mock_rpc from protobuf-rspec, already included in every example group. Verified the misnamed-constant mutation still produces the same 6 failures. Co-Authored-By: Claude <noreply@anthropic.com>
Owner
|
This pull request is quite large, so it's taking longer than anticipated to review. In the meantime, all of the specs pass with Active Model 8.1, so I've cut an alpha version (8.1.0.alpha) that can be used to start testing. |
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
2 participants
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.
Rails 8.1 support.