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 register method from Calculator #7001

Merged
merged 1 commit into from Feb 16, 2016

Conversation

nishant-cyro
Copy link
Contributor

We are not using this method anymore. Now, we use to register calculators like this -

config = Rails.application.config
config.spree.calculators.tax_rates << CustomCalculator
config.spree.calculators.shipping_methods << CustomCalculator
config.spree.calculators.promotion_actions_create_adjustments << CustomCalculator

@Mafi88
Copy link
Contributor

Mafi88 commented Feb 5, 2016

It seems to be unused both in code and according to https://github.com/spree/spree/blob/master/guides/content/developer/core/calculators.md.

However, shipment guide still mentions it as an example: https://github.com/spree/spree/blob/master/guides/content/developer/core/shipments.md#default-calculators.
And won't it brake existing extensions?

@nishant-cyro @priyank-gupta @damianlegawiec Any thoughts about it?

@nishant-cyro
Copy link
Contributor Author

@Mafi88 - Even if register method is there, it is not doing anything, so the extensions won't be working as expected.

Also, as mentioned in the guides for spree_active_shipping is outdated as spree_active_shipping is not using it too. https://github.com/spree/spree/blob/master/guides/content/developer/core/shipments.md#default-calculators

@Mafi88
Copy link
Contributor

Mafi88 commented Feb 11, 2016

👍

priyank-gupta added a commit that referenced this pull request Feb 16, 2016
Remove register method from Calculator
@priyank-gupta priyank-gupta merged commit f8a7ab4 into spree:master Feb 16, 2016
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

3 participants