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

fix: use legacy server as default with additional warnings #204377

Merged
merged 8 commits into from
Feb 6, 2024

Conversation

deepak1556
Copy link
Collaborator

@deepak1556 deepak1556 commented Feb 5, 2024

@deepak1556 deepak1556 self-assigned this Feb 5, 2024
@deepak1556 deepak1556 added this to the February 2024 milestone Feb 5, 2024
isidorn
isidorn previously approved these changes Feb 5, 2024
@isidorn
Copy link
Contributor

isidorn commented Feb 5, 2024

As @deepak1556 pointed out there are layering issues

So it looks like the changes we did in common/remote.contribution.ts should be in browser/remote.contribution.ts.
I understand that is a bigger change.

@bpasero @alexdima @aeschli can you help with the layering warnings? Suggestions? We could just ignore for the recovery release.

[00:52:20] /Users/demohan/github/vscode/src/vs/workbench/contrib/remote/common/remote.contribution.ts: line 32, col 32, Warning - Bad layering. You are not allowed to access browser from here, allowed layers are: [common] (local/code-layering)
/Users/demohan/github/vscode/src/vs/workbench/contrib/remote/common/remote.contribution.ts: line 32, col 32, Warning - Imports violates 'vs/base/common/** or vs/base/parts/*/common/** or vs/platform/*/common/** or vs/editor/common/** or vs/editor/contrib/*/common/** or vs/workbench/common/** or vs/workbench/services/*/common/** or vs/workbench/contrib/*/common/** or vscode-notebook-renderer or vs/nls or vs/amdX' restrictions. See https://github.com/microsoft/vscode/wiki/Source-Code-Organization (local/code-import-patterns)
/Users/demohan/github/vscode/src/vs/workbench/contrib/remote/common/remote.contribution.ts: line 34, col 30, Warning - Bad layering. You are not allowed to access browser from here, allowed layers are: [common] (local/code-layering)
/Users/demohan/github/vscode/src/vs/workbench/contrib/remote/common/remote.contribution.ts: line 34, col 30, Warning - Imports violates 'vs/base/common/** or vs/base/parts/*/common/** or vs/platform/*/common/** or vs/editor/common/** or vs/editor/contrib/*/common/** or vs/workbench/common/** or vs/workbench/services/*/common/** or vs/workbench/contrib/*/common/** or vscode-notebook-renderer or vs/nls or vs/amdX' restrictions. See https://github.com/microsoft/vscode/wiki/Source-Code-Organization (local/code-import-patterns)

@bpasero
Copy link
Member

bpasero commented Feb 5, 2024

I think the warning can be ignored if you are certain that the common pieces are not loaded into node.js environments (which I think here they are not).

But I also think the contribution could be moved over to the browser layer without much issues.

@deepak1556
Copy link
Collaborator Author

deepak1556 commented Feb 5, 2024

Moved it to browser layer, thanks @bpasero

@deepak1556 deepak1556 marked this pull request as ready for review February 6, 2024 01:53
@deepak1556 deepak1556 enabled auto-merge (squash) February 6, 2024 01:55
@deepak1556 deepak1556 disabled auto-merge February 6, 2024 02:35
@deepak1556 deepak1556 force-pushed the robo/revert_server_glibc_requirements branch from 44cc6b1 to 4b53bfa Compare February 6, 2024 03:12
@deepak1556 deepak1556 force-pushed the robo/revert_server_glibc_requirements branch from 4b53bfa to 7bcd639 Compare February 6, 2024 09:17
@deepak1556 deepak1556 requested a review from alexdima February 6, 2024 09:17
@deepak1556 deepak1556 enabled auto-merge (squash) February 6, 2024 09:41
@deepak1556 deepak1556 merged commit cf7ddbb into main Feb 6, 2024
6 checks passed
@deepak1556 deepak1556 deleted the robo/revert_server_glibc_requirements branch February 6, 2024 10:26
Copy link

@KidiIT KidiIT left a comment

Choose a reason for hiding this comment

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

Reviewed and tested all the items and found no signs of any damage to the unit or anything else that may have occurred during the installation of the system or the unit itself and all other components of the system that were installed in the unit as needed for the job or any damage to the home or any other activity in the unit.🫶❤️

deepak1556 added a commit that referenced this pull request Feb 6, 2024
* ci: switch to glibc 2.17 remote server

* chore: signal user about unsupported connection

* chore: address review comments

* chore: update nodejs build

* chore: bump distro

* chore: lower the minimum requirements

* fix: glibc version check

* chore: remove explicit connection disposal
deepak1556 added a commit that referenced this pull request Feb 6, 2024
…204482)

* fix: use legacy server as default with additional warnings (#204377)

* ci: switch to glibc 2.17 remote server

* chore: signal user about unsupported connection

* chore: address review comments

* chore: update nodejs build

* chore: bump distro

* chore: lower the minimum requirements

* fix: glibc version check

* chore: remove explicit connection disposal

* chore: update distro
@microsoft microsoft locked and limited conversation to collaborators Jun 11, 2024
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.

5 participants