Node issue #1140: Fix incorrect dispatch of vm.runInContext for argument "filename" #1141
Conversation
… argument "filename"
|
Is there a test we can do to show this broken? |
|
Hi ry - I have added a test case for this issue as well as updating the documentation section to the best of my understanding - please give it a read over. As far as I can see, there is some redundancy in this API area and it is not quite clear which are the blessed methods that will be supported going forwards. For example, all of the runInXxxx methods are available both as vm.runInXxxx as well as vm.Script.runInXxxx, and I notice that an alternative means of generating a fresh context object is available as new process.binding('evals').Context(). Could you confirm that the preferred way of accessing all of these script primitives is going to be through the vm.runInXxxx style? Cheers |
|
@ry - would you like to see anything else on this issue before you would be happy to commit it? |
|
LGTM otherwise. Last point of order: can you sign the CLA? |
|
Hi bnoordhuis - CLA signed, and I have updated my pull as per comments in this thread - thanks for the review. Let me know if there is anything more I can do |
|
Merged in c05936c. Thanks, Antranig. |
|
Many thanks, Ben, your attention is much appreciated :) |
This resolves issue at
#1140