-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Update cesiumWorkerBootstrapper.js #9573
Conversation
Proposed change enables Cesium to work with Electron. Alters scope of variables: requirejs, require, define This takes care of an issue as noted by Stuart Attenborrow in 2018. See at: https://community.cesium.com/t/fixing-cesiumworkerbootstrapper-when-using-electron-react-webpack/6532
Thank you so much for the pull request @CraigPatakyEDX! I noticed this is your first pull request and I wanted to say welcome to the Cesium community! The Pull Request Guidelines is a handy reference for making sure your PR gets accepted quickly, so make sure to skim that.
Reviewers, don't forget to make sure that:
|
Thanks again for your contribution @CraigPatakyEDX! No one has commented on this pull request in 90 days. Maintainers, can you review, merge or close to keep things tidy? I'm going to re-bump this in 90 days. If you'd like me to stop, just comment with |
I forgot if there was something else I was supposed to do with this
request. I know that it is a very useful request, and that electron will
not run with cesium without it.
…On Thu, Aug 26, 2021 at 7:00 PM Cesium Concierge ***@***.***> wrote:
Thanks again for your contribution @CraigPatakyEDX
<https://github.com/CraigPatakyEDX>!
No one has commented on this pull request in 90 days. Maintainers, can you
review, merge or close to keep things tidy?
*I'm going to re-bump this in 90 days. If you'd like me to stop, just
comment with @cesium-concierge stop. If you want me to start again, just
delete the comment.*
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#9573 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AST3MU5BDKW3DYCN4MPRCMDT63WTVANCNFSM45XRNDYA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
Thanks for the PR @CraigPatakyEDX. In order to merge you're fix, you need to do a few things:
@ebogo1 As of #9718, nothing else is blocking this fix from going in, correct? |
Actually, taking a closer look at this, this does not have anything to do with #9718. The line altered is part of a verbatim copy of requireJS, and should not be modified. I do believe that there may be a fix needed or further configuration to get Cesium to play nicely with Electron across the board, however I don't think this is the right way to go about it. @CraigPatakyEDX If you're still interested in addressing the cause please open an issue with more details on how to replicate, and we can figure out the best way to resolve it. |
Proposed change enables Cesium to work with Electron. Alters scope of variables: requirejs, require, define
This takes care of an issue as noted by Stuart Attenborrow in 2018.
See at: https://community.cesium.com/t/fixing-cesiumworkerbootstrapper-when-using-electron-react-webpack/6532