Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

"Support for WebWorker." Fixing missing self definition which fails linting #61

Closed
wants to merge 8 commits into from

Conversation

dryajov
Copy link
Member

@dryajov dryajov commented Jan 21, 2017

This fixes linting issues in PR #59

Tom Swindell and others added 2 commits January 14, 2017 12:45
Signed-off-by: Tom Swindell <t.swindell@rubyx.co.uk>
@dryajov
Copy link
Member Author

dryajov commented Jan 21, 2017

@diasdavid @dignifiedquire Could we merge this in? I'd love to get this in master so we can get it running in ServiceWorkers.

@dryajov dryajov changed the title fixing missing self definition which fails linting "Support for WebWorker." Fixing missing self definition which fails linting Jan 21, 2017
Copy link
Member

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

Can we get a test for the browser that checks it is available within a WebWorker?

(my previous comment: #59 (comment))

@dryajov
Copy link
Member Author

dryajov commented Jan 23, 2017

It seems like it would be better adding WebWorker tests to all the js projects... Its possible to do that with a karma plugin like this one - https://github.com/Joris-van-der-Wel/karma-mocha-webworker. I can bolt it as a one off to this project, but it'd be a hack.

Not sure how you guys go about making this sort of decisions, but I'd be happy to help with anything I can.

I've got a PR (ipfs/aegir#94) in aegir that addresses this issue. It's a POC, but seems to work as intended on this project at least.

@daviddias daviddias added the status/deferred Conscious decision to pause or backlog label Jan 23, 2017
@dignifiedquire
Copy link
Member

@dryajov thank you, lets get the aegir PR shipped and then we can ship this.

@dryajov
Copy link
Member Author

dryajov commented Jan 25, 2017

@diasdavid @dignifiedquire Can we get this PR merged - ipfs/aegir#94 (unless there is something else outstanding) so we can merge this changes in?

Also, we should be able to move the rest of the projects to running tests in WebWorkers once that's in, not sure how you want to track that effort.

@dignifiedquire
Copy link
Member

Thanks, will run this through in the morning and hopefully ship then.

Let's open an issue on ipfs/js-ipfs for tracking.

"Yusef Napora <yusef@napora.org>",
"dryajov <dryajov@gmail.com>",
Copy link
Member

Choose a reason for hiding this comment

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

Can you drop these two contributor additions, they are auto generated on release

"test:node": "aegir-test --env node",
"test:no-webcrypto": "NO_WEBCRYPTO=true aegir-test --env node",
"test:browser": "aegir-test --env browser",
"test:webworker": "aegir-test --env webworker",
Copy link
Member

Choose a reason for hiding this comment

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

Need the --env webworker for the releases as well

Copy link
Member Author

Choose a reason for hiding this comment

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

Is that needed? Looking at aegir, the tests tasks already get invoked on release and I've added the test:webworker task there.

@dignifiedquire dignifiedquire mentioned this pull request Jan 27, 2017
@dignifiedquire
Copy link
Member

Moved to #63

@dignifiedquire dignifiedquire removed the status/deferred Conscious decision to pause or backlog label Jan 27, 2017
@dignifiedquire
Copy link
Member

Just released :)

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

Successfully merging this pull request may close these issues.

3 participants