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

[eric-christian]

Hello :)

Describe the bug
Today we encountered a situation where two gems are patching the same core class with the same method. badum-ching
They Adyen 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:in const_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):

• ruby Version: 2.6.3
• adyen gem Version: 4.1.0
• aquarium gem Version: 0.7.1

Additional context
A quick search didn't reveal any usage of to_camel_case. Is it actually used? Can we simply remove it?
I'll also open an issue at the aquarium gem.

Best regards,
Eric

[hbkwong]

Hey Eric, good catch on this one! I see that the people maintaining the other gem's repo have been working on their own fix based on the Issue you'd raised as well.

I also do agree that we're shouldn't monkeypatch core classes directly (we even fixed another instance of this while back when HashWithAccessors was created to undo a change to the core Hash class).

As far as I know, to_camel_case just exists in a single file: ./lib/adyen-ruby-api-library.rb. @crrood After a quick grep, it looks like the only instance of to_camel_case is just in the definition. Do you know if we're using it elsewhere?

[crrood]

Hi guys, It was originally used to convert between actions and method names, however it looks like Dana removed it during the same change you mentioned: a74d2d9 <a74d2d9> According to that commit she removed it from adyen-ruby-api-library.rb as well, so it must have gotten re-added in a merge somewhere. Agreed that we shouldn’t be modifying core classes - let’s get rid of it. -- Colin Rood Contact: adyen.com/contact <http://adyen.com/contact> Adyen San Francisco 274 Brannan St. Suite #600

…

On May 20, 2020, at 5:51 PM, Andrew W. ***@***.***> wrote: Hey Eric, good catch on this one! I see that the people maintaining the other gem's repo have been working on their own fix based on the Issue you'd raised as well. I also do agree that we're shouldn't monkeypatch core classes directly (we even fixed another instance of this while back when HashWithAccessors was created to undo a change to the core Hash class). As far as I know, to_camel_case just exists in a single file: ./lib/adyen-ruby-api-library.rb. @crrood <https://github.com/crrood> After a quick grep, it looks like the only instance of to_camel_case is just in the definition. Do you know if we're using it elsewhere? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#67 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAUDLIYGAEZUM7RBSYAXBKTRSR3JNANCNFSM4NGBBOSQ>.

[eric-christian]

Hi guys,

that sounds awesome. :)

Best regards,
Eric

[crrood]

Hi Eric,

The method has been removed in #70.

Cheers,
Colin

[hbkwong]

Thanks Colin