Agilefreaks / Aquarium

An AOP library for Ruby
https://rubygems.org/gems/aquarium/
149 stars 24 forks source link

The `String.to_camel_case` patch collides with the patch from another gem #46

Closed eric-christian closed 4 years ago

eric-christian commented 4 years ago

Hello :)

Describe the bug Today we encountered a situation where two gems are patching the same core class with the same method. badum-ching The adyen-ruby-api-library gem and the aquarium gem are both patching the method to_camel_case into String. While both seem to have the same intention, the behavior varies slightly regarding the first character. One gem wants it as upper-case, the other one as lower-case.

We end up with the current exception: aquarium/aspects/advice.rb:281:inconst_get': wrong constant name noAdviceChainNode (NameError)`

Notice the lower-case 'n' in the name of a const.

Expected behavior The expected behavior would be to not have the method to_camel_case in the core String class at all. It would be best not to patch core classes at all within a gem. You never know who else might come up with the same idea for a method name.

Desktop (please complete the following information):

Additional context A quick search revealed only one usage of to_camel_case inside aquarium/aspects/advice.rb. Is that correct? Is it possible to move the definition of to_camel_case in to that same file? Or in to some kind of aquarium/utils.rb maybe? I'll also open an issue at the adyen gem.

Best regards, Eric

deanwampler commented 4 years ago

Thanks for raising this. It was pretty dumb of me to monkey patch String like this. I only use this call in one place! I'll create a new release tonight that removes the addition of to_camel_case to String.

deanwampler commented 4 years ago

I just pushed version 0.7.2 to rubygems.org, which should fix this issue.

eric-christian commented 4 years ago

Hi @deanwampler,

thank you for your work. You are awesome! :)

Best regards, Eric

deanwampler commented 4 years ago

Good luck!