-
Notifications
You must be signed in to change notification settings - Fork 2.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
normalize base of language filter pattern to support clients on another OS #8268
Conversation
56a4ee4
to
2772011
Compare
@eclipse-theia/ecd-theia-committers Can someone with Windows machine help to verify? Backend should run on Linux (you can just use Gitpod). |
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.
Tested with che-theia inside CRC (linux) and browser on Windows. "Debug Test | Run Test" code lenses show up now.
@@ -40,7 +40,6 @@ import { | |||
MarkerSeverity, DocumentLink, WorkspaceSymbolParams, CodeAction, CompletionDto | |||
} from '../../common/plugin-api-rpc-model'; | |||
import { RPCProtocol } from '../../common/rpc-protocol'; | |||
import { fromLanguageSelector } from '../../plugin/type-converters'; |
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.
This method is now unreferenced. Can we remove it? It looks kinda fishy anyway.
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.
removed
It also removes dependencies to the plugin process code from the main process. Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
2772011
to
6021f3a
Compare
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 unfortunately do not have a windows machine to test the functionality, but codewise it looks good to me 👍
Update: I also ran the tests and codelenses on Linux and it worked successfully.
Code wise looks good, Anyone with a window environment can perform a last check before merging for the release would be appreciated. Thanks |
Thomas already checked on windows against linux: #8268 (review) |
Good, You should merge it |
What it does
How to test
Extensions
viewReview checklist
Reminder for reviewers