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

Bump removal horizon for 3.x deprecations #4025

Merged

Conversation

kennyadsl
Copy link
Member

Description

Now that we are close to the release of 3.0, we need to change the deprecation horizon of our Spree::Deprecation class in order to print the right messages to users when we emit a deprecation warning.

This change will tell users that code deprecated using Spree::Deprecation will be removed in Solidus 4.0.

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)
  • I have attached screenshots to this PR for visual changes (if needed)

Now that we are close to the release of 3.0, we need to
change the deprecation horizon of our Spree::Deprecation
class in order to print the right messages to user when
we emit a deprecation warning.

This change will tell users that code deprecated using
Spree::Deprecation will be removed in Solidus 4.0.
@kennyadsl kennyadsl added the changelog:solidus_core Changes to the solidus_core gem label Apr 13, 2021
@kennyadsl kennyadsl added this to the 3.0.0 milestone Apr 13, 2021
@kennyadsl kennyadsl self-assigned this Apr 13, 2021
@spaghetticode
Copy link
Member

@kennyadsl I'm wondering if we can write some code that will remove the need to periodically increment that number manually.

I'm not sure how viable this is (is there any exception to the rule?), but maybe something like this may work:

  Deprecation = begin
    version = Spree.solidus_gem_version.prerelease? ? Spree.solidus_gem_version.segments.first : Spree.solidus_gem_version.segments.first + 1
    ActiveSupport::Deprecation.new("#{version}.0" , 'Solidus')
  end

@kennyadsl
Copy link
Member Author

@spaghetticode good idea, I was thinking that just picking the next major would be enough though. Why we'd need the prerelease check?

@spaghetticode
Copy link
Member

@kennyadsl I was inspired by the current behavior, I mean before this PR. We're on 3.0.0.rc2 but the deprecation horizon is still 3.0... I guessed it actually made sense since we're not officially on 3.0 yet.

It's rather unlikely, but what would happen if we needed to introduce a new deprecation today? Would it need to be removed before the 3.0 final release or it would remain there until 4.0?

@kennyadsl
Copy link
Member Author

kennyadsl commented Apr 14, 2021

Got it. I think we should have done this before, actually. Version 3.0 should not contain any deprecation warning but only remove existing ones. So I guess we can just use the next major as the deprecation horizon.

I'm wondering if we want to keep the deprecation cycle as is or we want to update it to reflect what Rails does (remove deprecated functionality in minor versions). This is also what we state here:

We would like to also follow the Rails approach: deprecating functionality in one minor version and removing it in the next.

even if we never applied this rule.

That said, until we end up with a decision, I'd keep it as a manual update.

@spaghetticode
Copy link
Member

Sure, we can automate later 👍

@kennyadsl kennyadsl merged commit e15c3d3 into solidusio:master Apr 14, 2021
@kennyadsl kennyadsl deleted the kennyadsl/bump-deprecation-horizon-3.0 branch April 14, 2021 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:solidus_core Changes to the solidus_core gem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants