Skip to content
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

WIP - Axe runner Electron adapter (Ace core uses Puppeteer instead) #42

Merged
merged 4 commits into from
May 5, 2019

Conversation

danielweck
Copy link
Member

Note that this is working with the current version of Electron (v2). We can upgrade to v5 in a separate PR, at a later date (at which point I would recommend updating many other NPM packages too, across the Ace board, in order to address security holes, use latest performance improvements, etc.).

Meanwhile, we can keep this PR narrow-focused.

I compared the JSON reports (using a text-level diff tool) generated by Electron (Ace GUI) and Puppeteer (Ace core CLI), both Chromium but different versions ... everything looks good.

@danielweck
Copy link
Member Author

Related PR: daisy/ace#227

@danielweck
Copy link
Member Author

Please note that packages dependencies in package.json and yarn.lock are necessarily screwed-up because of manually hacking the Ace core imports.

@danielweck danielweck requested a review from rdeltour May 2, 2019 22:30
@danielweck danielweck changed the title WIP - Electron Axe runner instead of Puppeteer (which is used by Ace core) WIP - Axe runner Electron adapter (Ace core uses Puppeteer instead) May 3, 2019
Copy link
Member

@rdeltour rdeltour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar enough with express and Electron to accurately review the details, but this looks all good to me! 🎉

@danielweck danielweck merged commit c1996ba into master May 5, 2019
@danielweck danielweck deleted the feature/axe-runner branch May 5, 2019 21:23
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