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

Deprecated delegate_belongs_to #8179

Merged
merged 3 commits into from Aug 11, 2017

Conversation

Punkbooster
Copy link
Contributor

No description provided.

@krzysiek1507
Copy link
Contributor

@Punkbooster please check all occurrences of those methods https://github.com/spree/spree/search?utf8=%E2%9C%93&q=delegate_belongs_to&type=

@Punkbooster Punkbooster force-pushed the deprecated_delegate branch 2 times, most recently from 67971f2 to 8d2315d Compare July 28, 2017 14:41
@@ -118,6 +113,22 @@ class Product < Spree::Base
self.whitelisted_ransackable_attributes = %w[description name slug discontinue_on]
self.whitelisted_ransackable_scopes = %w[not_discontinued]

[

Choose a reason for hiding this comment

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

Use %i or %I for an array of symbols.

@@ -26,6 +21,11 @@ def has_default_price?
!self.default_price.nil?
end


def find_or_build_default_price

Choose a reason for hiding this comment

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

Use empty lines between method definitions.

@@ -26,6 +21,11 @@ def has_default_price?
!self.default_price.nil?
end


Choose a reason for hiding this comment

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

Extra blank line detected.

@damianlegawiec damianlegawiec changed the title Remove deprecated delegate_belongs_to Deprecated delegate_belongs_to Aug 11, 2017
@damianlegawiec damianlegawiec self-requested a review August 11, 2017 13:34
@damianlegawiec damianlegawiec merged commit ccf237f into spree:master Aug 11, 2017
@ezmiller
Copy link

Is there a discussion about why this was deprecated somewhere?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants