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

Multiple improvements #65

Merged
merged 9 commits into from
Nov 16, 2017
Merged

Multiple improvements #65

merged 9 commits into from
Nov 16, 2017

Conversation

molant
Copy link
Contributor

@molant molant commented Oct 6, 2017

  • Added appveyor to generate the binaries automatically on the cloud and publish them (unsigned) to the GitHub releases
  • Create binaries for x86 and x64 for node 6 and 8
  • Update dependencies to latest versions
  • Add support for node-pre-gyp so people in the right platform don't need to install anything (will get binaries from GitHub releases)
  • Updated documentation
  • Updated version to 0.6.0-unsigned so it's clear what's happening
  • Fix issues installing the VC++ redistributable in x86 node

@molant
Copy link
Contributor Author

molant commented Oct 6, 2017

I still need to update the appveyor.yml file to deploy only on tag or in a certain branch (still haven't decided) and also see if I need to update the prebuild.cmd (haven't been capable of building locally yet...).

This PR should take care of #18 (fixed in a previous PR), #34 (haven't seen any of those warnings in my tests), #47, and #56 that I'm aware.

package.json Outdated
"description": "Microsoft Edge Diagnostics Adapter",
"main": "out/src/edgeAdapter.js",
"main": "dist/edgeAdapter.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

that file doesn't exist, it should it should be like it was before out/src/edgeAdapter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I though I reverted this changed :/

@molant molant force-pushed the update-dependencies branch 5 times, most recently from c307ed4 to 6b029f4 Compare October 9, 2017 22:25

this._httpServer = http.createServer((req, res) => this.onServerRequest(req, res));
this._webSocketServer = new WebSocketServer({ server: this._httpServer });
this._webSocketServer.on('connection', (client) => this.onWSSConnection(client));
Copy link
Contributor

Choose a reason for hiding this comment

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

you need to add a second parameter to the event handler. Something like this:
this._webSocketServer.on('connection', (client, message) => this.onWSSConnection(client, message));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed and protocol.json updated. I'll need to manually update the release with the new files though

@sarvaje
Copy link
Contributor

sarvaje commented Nov 13, 2017

@molant right now there is an error if you launch npm run build because the env variable VCTargetsPath is not defined.

To make it works, you need to run first the prebuild.cmd. is it possible to add the value of that variable to the command npm run build?

@molant
Copy link
Contributor Author

molant commented Nov 14, 2017

@sarvaje, done!

@molant molant force-pushed the update-dependencies branch 4 times, most recently from c56453d to e328cb0 Compare November 14, 2017 05:30
@molant molant merged commit 41cb47c into master Nov 16, 2017
@molant
Copy link
Contributor Author

molant commented Nov 16, 2017

Finished doing all the testing and things are working on a clean install of RS2. Merging!

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.

None yet

2 participants