Remove unused ShippingRateTaxer service object
#4136
Conversation
|
@solidusio/core-team can I get one more review on this? 🙏 |
This class was originally used by `Spree::Stock::Estimator` to apply taxes to a shipping rate, much like `Spree::Tax::OrderAdjuster` does for orders. The only call site was then removed in 3639585[1] with the goal of improving the performance of shipping rate estimation, but the actual service object was not removed, and is currently sitting unused in the codebase. [1]: 3639585
aldesantis
force-pushed
the
remove-shipping-rate-taxer
branch
from
479ebb4
to
73e7470
Compare
|
@aldesantis I understand this class has not been used for a long time in Solidus codebase, but I'm wondering should we deprecate it before removing it? |
|
@spaghetticode I gave it some consideration and I'm not totally sure. I'm sure someone might be using it for some reason but... what reason exactly? The class is not used by Solidus in any way, so I guess the only situation is if someone overrode the shipping rate estimator and their custom estimator uses I think when you see it this way, the class is a public API and should be deprecated. On the other hand, it also feels weird to deprecate a class that has no call sites in Solidus. |
|
I'm also fine with removing this since it's not in use. I don't see a benefit to deprecating an orphaned class. If it breaks something for some hypothetical person who decided to build off of an interrnal class we aren't using then their test suite should catch it and they can copy the class into their codebase. |
|
We posted a message about this in the Solidus Slack and no one objected, so I'm merging! |
Description
The
Spree::Tax::ShippingRateTaxerservice object is not being used anywhere. Its original purpose was to apply taxes to estimated shipping rates, but that logic was moved intoSpree::Stock::Estimatorin 3639585 for performance reasons.The only call site was removed, but the service object itself was not, and it's currently sitting unused in the codebase (I found out about it while writing the Tax calculation guide).
Checklist: