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

Remove unused methods on ActiveStorage::Variant #49196

Merged
merged 1 commit into from Sep 8, 2023

Conversation

p8
Copy link
Member

@p8 p8 commented Sep 8, 2023

These methods were added in b221a4d But they don't seem to be used by Rails internally or have any tests, so I assume they were added by accident?
As they both seem to be marked as :nodoc: on ActiveStorage::Blob, we can remove them without a deprecation warning.

If we decide to keep these methods, they should be added to ActiveStorage::VariantWithRecord as well.
No one complaining about there methods missing on ActiveStorage::VariantWithRecord is another reason these methods aren't used.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

These methods were added in b221a4d
But they don't seem to be used by Rails internally or have any tests, so
I assume they were added by accident?
As they both seem to be marked as :nodoc: on ActiveStorage::Blob, we can
remove them without a deprecation warning.

If we decide to keep these methods, they should be added to
ActiveStorage::VariantWithRecord as well. No one complaining about there
methods missing on ActiveStorage::VariantWithRecord is another reason
these methods aren't used.
@p8 p8 force-pushed the activestorage/remove-unused-methods branch from fe6bf74 to 1e23fff Compare September 8, 2023 08:06
@kamipo kamipo merged commit 9b00c65 into rails:main Sep 8, 2023
4 checks passed
@p8 p8 deleted the activestorage/remove-unused-methods branch September 8, 2023 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants