-
Notifications
You must be signed in to change notification settings - Fork 221
Update weborker code to work with te CDN #1285
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
base: develop
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1285 +/- ##
===========================================
+ Coverage 86.69% 86.72% +0.02%
===========================================
Files 337 337
Lines 84107 83955 -152
Branches 3126 4750 +1624
===========================================
- Hits 72919 72812 -107
+ Misses 11188 11120 -68
- Partials 0 23 +23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment.
Otherwise this looks great. I particularly like that the LiteAdaptor
, in particular LiteElement
, is no longer dependent on mathjax
at all.
const file = self.SREfeature.json + '/' + locale + '.json'; | ||
return fetch(file).then((data) => data.json()).catch((err) => console.log(err)); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this happen in a separate method, as this is the only part that depends on the shape of the SRE configuration object, so it is clear where it needs to be changed in case that is changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've move the SREfeature changes to within the speech-worker.ts
file, so all the SRE-related stuff is there. Does that resolve this issue for you?
Yes, I am glad to be able to remove that code as well. It was never ideal, but worked for what we needed. This is much cleaner, so for that reason, I'm glad we ran into the problem and were able to fix it in a better way. |
This PR resolves the problems we are having with using the worker code from the CDN. It rurns out that jsdelivr won't serve the HTML file for the worker pool (it wraps it in al in a