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

Add back Variant#find_or_build_default_price #4960

Conversation

spaghetticode
Copy link
Member

Summary

This method was renamed to default_price_or_build with 6420b10, but this was a breaking change given it was released with Solidus 3.1. This commit reintroduces the method as an alias but with deprecation.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

@spaghetticode spaghetticode self-assigned this Mar 1, 2023
@spaghetticode spaghetticode requested a review from a team as a code owner March 1, 2023 15:55
@github-actions github-actions bot added the changelog:solidus_core Changes to the solidus_core gem label Mar 1, 2023
@spaghetticode spaghetticode force-pushed the add-back-variant-find_or_build_default_price branch from d8df32b to ae37846 Compare March 1, 2023 16:07
@codecov
Copy link

codecov bot commented Mar 1, 2023

Codecov Report

Merging #4960 (65a6f2d) into master (6d55800) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #4960   +/-   ##
=======================================
  Coverage   86.69%   86.69%           
=======================================
  Files         578      578           
  Lines       14681    14684    +3     
=======================================
+ Hits        12728    12731    +3     
  Misses       1953     1953           
Impacted Files Coverage Δ
core/app/models/concerns/spree/default_price.rb 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@kennyadsl kennyadsl added type:bug Error, flaw or fault backport-v3.2 backport-v3.3 Backport this pull-request to v3.3 labels Mar 1, 2023
def find_or_build_default_price
default_price_or_build
end
deprecate :find_or_build_default_price, deprecator: Spree::Deprecation
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
deprecate :find_or_build_default_price, deprecator: Spree::Deprecation
deprecate :find_or_build_default_price, :default_price_or_build, deprecator: Spree::Deprecation

👆 I think this will generate a better deprecation message.

Copy link
Contributor

Choose a reason for hiding this comment

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

We're trying to get better with yard documentation, so here we have a nice opportunity to use the @deprecated tag: https://rubydoc.info/gems/yard/file/docs/Tags.md#deprecated 🙂

Copy link
Contributor

@waiting-for-dev waiting-for-dev left a comment

Choose a reason for hiding this comment

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

Thanks, @spaghetticode! I left one suggestion.

def find_or_build_default_price
default_price_or_build
end
deprecate :find_or_build_default_price, deprecator: Spree::Deprecation
Copy link
Contributor

Choose a reason for hiding this comment

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

We're trying to get better with yard documentation, so here we have a nice opportunity to use the @deprecated tag: https://rubydoc.info/gems/yard/file/docs/Tags.md#deprecated 🙂

This method was renamed to `default_price_or_build` with 6420b10, but
this was a breaking change given it was released with Solidus 3.1.
This commit reintroduces the method as an alias but with deprecation.
@spaghetticode spaghetticode force-pushed the add-back-variant-find_or_build_default_price branch from ae37846 to 65a6f2d Compare March 2, 2023 11:41
@spaghetticode
Copy link
Member Author

@kennyadsl @waiting-for-dev thanks for the precious suggestions ❤️

Copy link
Contributor

@waiting-for-dev waiting-for-dev left a comment

Choose a reason for hiding this comment

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

Lovely, @spaghetticode!

@github-actions
Copy link

github-actions bot commented Mar 2, 2023

💚 All backports created successfully

Status Branch Result
v3.2
v3.1
v3.3

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

waiting-for-dev added a commit that referenced this pull request Mar 2, 2023
[v3.3] Merge pull request #4960 from spaghetticode/add-back-variant-find_or_build_default_price
kennyadsl added a commit to nebulab/solidus that referenced this pull request Mar 17, 2023
kennyadsl added a commit to nebulab/solidus that referenced this pull request Mar 21, 2023
kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 12, 2023
kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 14, 2023
kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 18, 2023
kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v3.3 Backport this pull-request to v3.3 changelog:solidus_core Changes to the solidus_core gem type:bug Error, flaw or fault
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants