-
Notifications
You must be signed in to change notification settings - Fork 84
Conversation
…have ill-defined semantics.
👎 on re-implementing this on Ridley. It already exists in https://github.com/RiotGames/buff-extensions. We should figure out why it's not being picked up correctly. |
@sethvargo The issue is that if this changes so that we get the buff-extensions behavior, anything using the other behavior will break if they happen to require ridley. |
Then we should fix buff to match |
Using a core monkeypatch to polyfill features the user might not have is one thing, but for a library to invent new core semantics seems like a really bad idea. There is no common agreement on this that I can find. |
Maybe have buff-extensions offer its core_exts as module methods too so they can be called directly rather than used via monkey patch? |
Sure. But I think this is the wrong solution. |
Looking at this some more, I don't see where in ridley it requires |
@coderanger thats weird, it is intended that Ridley leverages buff-extensions. I think we should ensure it's required and stick with that instead of re-implementing the slice function in Ridley itself. We can also update buff-extensions to allow the functions to be called as class methods on the module instead of just mixins. |
I ran into this as well when I tried to upgrade Vagrant 1.5.4 and vagrant-berkshelf 2.0.1 |
This seems to be bubbling up to schubergphilis/vagrant-chef-zero#51 also.
|
+1 having an issue with this |
@coderanger thanks for providing this and sorry for sitting on it for so long |
Hash#slice, at least for me, is coming from the i18n gem (by way of Vagrant) and this seems to have different semantics from what the Ridley code expects. Specifically it appears the buff-extensions version won't throw an error if a given key isn't found, while with i18n it will. In general having a library that relies on non-standardized core extensions is :-(
The original traceback is: