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

fix worker instance misconfiguration #261

Closed
wants to merge 5 commits into from

Conversation

ratik
Copy link
Contributor

@ratik ratik commented Oct 24, 2014

Hi

I've added passing config into the worker. Could you please review.

Thanks

@tanx
Copy link
Member

tanx commented Oct 24, 2014

Thanks for the PR. After you add a unit test and adjust the documentation it will be merged

@tanx
Copy link
Member

tanx commented Dec 9, 2014

Any progress on this? Do we even need a unit test since this is in the tests anyway? thoughts?

@ratik
Copy link
Contributor Author

ratik commented Dec 9, 2014

Added the test. Not sure why it fails on that

* @param {String} path relative path to the worker scripts
*/
function initWorker(path) {
asyncProxy = new AsyncProxy(path);
asyncProxy = new AsyncProxy(path,this.config);
Copy link
Member

Choose a reason for hiding this comment

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

Sorry to be nitpicking. Please use a space between args here to keep the code styles consistent.

@tanx
Copy link
Member

tanx commented Dec 10, 2014

Thanks for getting back to the PR. I've looked over the code and all in all it looks good. Just made some minor remarks. Nothing major.

Added the test. Not sure why it fails on that

Mmmmh. Not sure what the issue is here either. This is what travis tells us:
https://travis-ci.org/openpgpjs/openpgpjs/builds/43507174#L686

Maybe debugging locally will help you to spot the underlaying issue?

@ratik
Copy link
Contributor Author

ratik commented Dec 10, 2014

ok, I've just got the test tools working (the were issues on MacOsX) on Ubuntu, so will fix it soon.

@tanx
Copy link
Member

tanx commented Feb 11, 2015

I manually merged this here da3dbf7

@tanx tanx closed this Feb 11, 2015
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