Skip to content

Modernisation with MsieJavaScriptEngine support #12

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ducas
Copy link

@ducas ducas commented Aug 22, 2013

Let's use a more modern JavaScript engine that has ECMAScript 5 polyfills...

@cbaxter
Copy link
Owner

cbaxter commented Aug 22, 2013

@ducas Thanks, I will review over the weekend.

@ducas
Copy link
Author

ducas commented Aug 22, 2013

No worries. Feel free to suggest better methods of integration. I wanted to
keep backwards compatibility so tried to fit in as seamlessly as possible.

Regards,
Ducas Francis

M: +61 402 228 855

On 22/08/2013, at 23:18, Chris Baxter notifications@github.com wrote:

@ducas https://github.com/ducas Thanks, I will review over the weekend.


Reply to this email directly or view it on
GitHubhttps://github.com//pull/12#issuecomment-23088755
.

@cbaxter
Copy link
Owner

cbaxter commented Aug 24, 2013

@ducas Overall, this will be a good enhancement.

I think the MsieJavaScriptEngine should be split out in to a separate assembly as an extension similar to JSTest.Integration.xUnit. With the MsieJavaScriptEngine implementation split out, I think that it may be useful to have a method of overriding the default engine factory property so that if you are using the MsieJavaScriptEngine that will become the default engine for the TestScript class (i.e., avoid the need for new TestScript(new MsieJavaScriptEngine());.

Thoughts?

@cbaxter
Copy link
Owner

cbaxter commented Sep 5, 2013

@ducas I will make a point of looking at this further over the weekend as time permits. Given the change in .NET Framework version, I will likely end up packaging this as a 2.0 release and as such may make some other small changes.

@cbaxter
Copy link
Owner

cbaxter commented Sep 10, 2013

@ducas Would you mind re-creating this PR targeting the new vNext branch? Thanks!

@ducas
Copy link
Author

ducas commented Sep 10, 2013

Sure, I'll give it a look over the next couple days.

Regards,
Ducas Francis

M: +61 402 228 855

On 10/09/2013, at 22:07, Chris Baxter notifications@github.com wrote:

@ducas https://github.com/ducas Would you mind re-creating this PR
targeting the new vNext branch? Thanks!


Reply to this email directly or view it on
GitHubhttps://github.com//pull/12#issuecomment-24154507
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants