-
Notifications
You must be signed in to change notification settings - Fork 479
copy the passed-in ruleset in verify(); fixes #458 #465
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
|
Did you have a chance to test it out with the Rhino CLI? Note that it does fall down for the full suite though due to heap issues. |
|
@nschonni No, I don't have Rhino. |
|
|
|
Okay, it seems to pass: |
|
Also, in case it wasn't already apparent, #458 is affecting us over at Bootstrap. |
|
@stubbornella I hate to be a bother, but: Ping. |
|
@cvrebert I'm going to be going through all the PRs and seeing what can go into a bugfix release vs the next point release. |
|
@stubbornella https://github.com/pvorb/node-clone/blob/master/clone.js looks to be safe vanilla JS that should work across the various runners. Thoughts on merging this? @cvrebert I've landed a few things recently, do you mind rebasing again? |
|
@nschonni Rebasing completed. |
|
👍 thanks |
|
@nschonni Is this still auto-mergeable without conflicts? |
|
Nope, I was going to manually merge since it was my fault that I made it unmergable after asking you to rebase 😊 |
since applyEmbeddedRuleset modifies the ruleset in-place, this ensures that those modifications aren't visible to the caller
|
@nschonni Freshly rebased. |
copy the passed-in ruleset in verify(); fixes #458
|
Praise |
Fixes #458 and adds a relevant unit test.
Passes both
grunt testand the in-browser test (in Chrome).