Shopify / deprecation_toolkit

⚒Eliminate deprecations from your codebase ⚒
MIT License
460 stars 40 forks source link

DeprecationToolkit adds private methods in the Minitest module - should we avoid that? #21

Closed rmacklin closed 5 years ago

rmacklin commented 5 years ago

These two methods: https://github.com/Shopify/deprecation_toolkit/blob/51692599f76f6b8748ea0868a271b271595c88e1/lib/minitest/deprecation_toolkit_plugin.rb#L23-L36

are defined inside the Minitest module. However, the methods themselves are independent. It seems safer to define them inside the DeprecationToolkit namespace, to keep this gem isolated from potential conflicts with other gems that could redefine Minitest#add_notify_behavior and Minitest#attach_subscriber.

If you agree, I'd be happy to open a PR.

Edouard-chin commented 5 years ago

That sounds reasonable to me 👍

rmacklin commented 5 years ago

Ok, I opened https://github.com/Shopify/deprecation_toolkit/pull/23