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 already deprecated methods in ActiveStorage #42598

Merged

Conversation

santib
Copy link
Contributor

@santib santib commented Jun 25, 2021

Summary

These methods were already deprecated on Rails 6.1, so I think it'd be appropriate to remove them for Rails 7.0

For reference, the deprecations where added here:
#34827
#37856

@p8
Copy link
Member

p8 commented Jun 25, 2021

Nice find!
Maybe we should add versions to that deprecate method to make them easier to find:
deprecate :build_after_upload, remove_in: '7.1'

@zzak
Copy link
Member

zzak commented Jun 25, 2021

Agree with @p8 I'm a bit surprised they were deprecated without mentioning the version they will be removed in 🤔

@pixeltrix
Copy link
Contributor

Agree with @p8 I'm a bit surprised they were deprecated without mentioning the version they will be removed in 🤔

@zzak they'll default to the value of ActiveSupport::Deprecation.deprecation_horizon which is handled in the ActiveSupport::Reporting::Reporting module.

@pixeltrix
Copy link
Contributor

@santib can you add a CHANGELOG entry, thanks - once you've added that I think we're good to go 👍🏻

@zzak
Copy link
Member

zzak commented Jun 26, 2021

@pixeltrix So do we normally update this value to the next version?

def initialize(deprecation_horizon = "7.1", gem_name = "Rails")

So if I had installed this gem at version 6.0, the horizon value would be 6.1?

Interesting if you look at 6.1 branch the value is a non-existant Rails version:
https://github.com/rails/rails/blob/6-1-stable/activesupport/lib/active_support/deprecation.rb#L41

Not sure how relevant this is tho, just sharing 🤔

@pixeltrix
Copy link
Contributor

Yes, I'm aware of the version mismatch on 6-1-stable. Not sure if it should change or not - I'm leaning towards it should (no-one will have asserted on that value in their test suite, right? 😅 ).

@santib santib force-pushed the remove-deprecated-methods-active-storage branch from d02bc13 to 65b1e1b Compare June 27, 2021 23:32
@santib
Copy link
Contributor Author

santib commented Jun 27, 2021

@santib can you add a CHANGELOG entry, thanks - once you've added that I think we're good to go 👍🏻

@pixeltrix I just added a CHANGELOG entry. LMK if you'd like to word it in some other way.

@pixeltrix pixeltrix merged commit 306449b into rails:main Jun 28, 2021
@pixeltrix
Copy link
Contributor

@santib thanks! 👍🏻

@kamipo
Copy link
Member

kamipo commented Jun 28, 2021

FYI usually removing all deprecated code for Rails 7.0 will be done by the release manager.

#34954
https://github.com/rails/rails/commits/main/guides/source/6_1_release_notes.md?author=rafaelfranca

@pixeltrix
Copy link
Contributor

@kamipo my understanding was that these were missed for removal in 6.1, apologies if I misunderstood that.

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

5 participants