search

solidusio / solidus_paypal_braintree

⛔️ [Archived] Use solidus_braintree instead!
https://github.com/solidusio/solidus_braintree
BSD 3-Clause "New" or "Revised" License
37 stars 78 forks source link

Do not safeguard including of Spree::Preferences::Persistable #316

Closed tvdeyen closed 1 year ago

tvdeyen commented 2 years ago

The safeguard prevents this module to be included because the class responds to preference (it is included in Spree::Base).

My guess is that this safeguard was included to avoid including Spree::Preferences::Preferable twice (it is included by Spree::Preferences::Persistable)

But Ruby will take care of not including the same module twice.

This will not effect user of Solidus >= 3.0, but helps to get rid of a (rather large) deprecation warning in Solidus 2.11

tvdeyen commented 2 years ago

Not so sure about Ruby not including things twice:

[9] pry(main)> module Foo
[9] pry(main)*   def self.included(klass)
[9] pry(main)*     puts "INCLUDED"
[9] pry(main)*   end  
[9] pry(main)* end  
=> :included
[10] pry(main)> include Foo
INCLUDED
=> Object
[11] pry(main)> include Foo
INCLUDED
=> Object
[12] pry(main)>

Ok, maybe I did not phrased it correctly. Ruby will not append the same methods to the same class twice. Also this is a ActiveSupport::Concern and this will take care of other potential side effects of the include.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.