-
Notifications
You must be signed in to change notification settings - Fork 30.7k
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
Environment service debt between common, browser, native #102918
Comments
Yeah, lot's of debt buried inside environment service, but I would hold on changing this until we figured out the sandbox model (#92164). I expect that the environment service will change a lot after this and I feel there is a chance to cleanup everything when we have an idea what this thing ends up to be. If we find out that the web API options are only used in the web version, then maybe they should be removed from the common environment service and only be accessible from the browser workbench environment service. I am not keen on moving the options into core though out of |
Sounds good, it is great to hear that this is being taken care of. Let's park (or drop) this PR for now. The problem with keeping the options inside |
From #76719: we carry around multiple properties for backups ( |
As one step to untangle the environment service monster, I introduced a //fyi @sandy081 |
There are
common
andbrowser
versions of the environment service but both of them depend on workbench.web.api.ts for IWorkbenchConstructionOptions. This is a type only dependency so it should not create any problems at runtime but it is misleading sincecommon
code gets access to browser-only interfaces such as IUpdateProvider.The culprit seems to be the unclear status of workbench.web.api inside
.eslintrc.json
.See https://github.com/microsoft/vscode/wiki/Source-Code-Organization#target-environments
The text was updated successfully, but these errors were encountered: