-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Make Raven.wrap respect prototype chain #402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@benvinegar This seems sane to me, but I'm not 100% sure. Thanks @jlfwong. 🍰 |
Interesting that this didn't come from the loop before it when copying all the properties over. This seems reasonable to me then. I'll cut a new version of this for you. :) |
Make Raven.wrap respect prototype chain
Also... not sure why Travis isn't running tests for Pull Requests. Then I'd have noticed this before merging. :/ Ugh. |
I'm inclined to think we can just remove that one test and it should be ok. |
Thanks for the merge! |
@jlfwong Can you address my comments above please? :/ |
I don't understand why the test to ensure the prototype isn't copied exists at all. It seems reasonable to me to just remove that test |
Thanks. That's my assumption as well. I'm just waiting on feedback from @benvinegar as a double check, and we'll be good here. Thanks again. :) |
Agreed. I think this patch is correct (hi @jlfwong). Hopefully this doesn't bite anyone randomly when upgrading. |
Thanks! (hi @benvinegar!) Is a new release going to be cut? |
Yeah, I'm going to review everything and will get it cut today. |
See #401
Disclaimer: I didn't clone the repo and test this, I'm just sorta hoping this works, and using this as clearer documentation of the problem in #401.