Skip to content
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

Handle attribute_method_matchers rename to attribute_method_patterns #811

Merged
merged 2 commits into from
Feb 11, 2022

Conversation

nvasilevski
Copy link
Contributor

Motivation

Tapioca is failing on Rails edge because of the method rename
https://github.com/rails/rails/pull/44367/files

Implementation

If I'm not mistaken, we can't to rely on any particular Rails version in this case so taking respond_to? approach similar to

target = if matcher.respond_to?(:method_missing_target)
# Pre-Rails 6.0, the field is named "method_missing_target"
T.unsafe(matcher).method_missing_target
else
# Rails 6.0+ has renamed the field to "target"
matcher.target
end

I'm also renaming references to values from matcher to pattern according to the new terminology. Let me know if you think this is unnecessary.

Tests

I'm going to run tests against Rails edge but I don't think there is any test I can add to existing ones?

@Morriar Morriar requested a review from a team February 11, 2022 00:47
@nvasilevski nvasilevski force-pushed the accomodate-rails-attribute_method_matchers-rename branch from 0b0ae07 to 677adc7 Compare February 11, 2022 00:53
@nvasilevski nvasilevski marked this pull request as draft February 11, 2022 00:55
@nvasilevski
Copy link
Contributor Author

Requires some work to properly handle AttributeMethodPattern constant rename

@nvasilevski nvasilevski force-pushed the accomodate-rails-attribute_method_matchers-rename branch 2 times, most recently from 374989b to d3a844f Compare February 11, 2022 20:33
@nvasilevski nvasilevski force-pushed the accomodate-rails-attribute_method_matchers-rename branch from d3a844f to d5316dd Compare February 11, 2022 20:33
@nvasilevski nvasilevski marked this pull request as ready for review February 11, 2022 20:34
Signed-off-by: Alexandre Terrasa <[email protected]>
Copy link
Collaborator

@Morriar Morriar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the liberty to push another commit to inline the, now shorter, signature on handle_method_pattern?.

Thanks 👍

@Morriar Morriar merged commit c52f42b into main Feb 11, 2022
@Morriar Morriar deleted the accomodate-rails-attribute_method_matchers-rename branch February 11, 2022 21:14
@Morriar Morriar added the bugfix label Feb 11, 2022
@shopify-shipit shopify-shipit bot temporarily deployed to production February 17, 2022 22:56 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to 0-7-stable March 29, 2022 19:23 Inactive
@paracycle paracycle added the backported Backported to stable branch label Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported Backported to stable branch bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants