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] feat(test): enable tests in webworkers by default #98

Merged
merged 2 commits into from
Feb 7, 2017

Conversation

dryajov
Copy link
Member

@dryajov dryajov commented Feb 3, 2017

No description provided.

@dignifiedquire
Copy link
Member

Thank you, will ship once I ran it through a couple of tests

@dryajov
Copy link
Member Author

dryajov commented Feb 6, 2017

@dignifiedquire I found a bug in this PR, and will be updating it today. Can we leave this for now and mark it as WIP?

Thanks!

@dignifiedquire
Copy link
Member

Sure thing, thanks

@dignifiedquire dignifiedquire changed the title feat(test): enable tests in webworkers by default [WIP] feat(test): enable tests in webworkers by default Feb 6, 2017
@dryajov
Copy link
Member Author

dryajov commented Feb 7, 2017

@dignifiedquire made files and frameworks config entries for karma.conf.js dynamic based on --dom/--webworker parameters. Let me know what you think.

@dignifiedquire
Copy link
Member

Lgtm @dryajov is this ready now?

@dryajov
Copy link
Member Author

dryajov commented Feb 7, 2017

@dignifiedquire It should be. My only concern is that releasing this will break projects that fail under WW tests, until this PRs are merged:

But this are failing on lint because of missing self definition.

One solution would be to release this as a major version, and then upgrade the rest of the projects once those PRs are merged int. The ones that are failing on a missing self definition, can be temporarily fixed with /* global self */.

Let me know how you want to proceed with this. I just don't want to leave things in a broken state and this will, until the rest of the PRs are merged in.

@dryajov
Copy link
Member Author

dryajov commented Feb 7, 2017

I should clarify that, the self issue is being addressed by this PR, so its kind of a "chicken and egg" problem, as mentioned above, using /* global self */ on those Prs temporarily should break the circular dependency.

@dignifiedquire
Copy link
Member

I will make this a major version bump anyway, so we can upgrade the dependencies one by one once they are ready, without breaking them. That way we don't have to add the global comments in there.

@dignifiedquire dignifiedquire merged commit 233708c into ipfs:master Feb 7, 2017
@dignifiedquire
Copy link
Member

published v10

@dryajov
Copy link
Member Author

dryajov commented Feb 7, 2017

awesome 🎈

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