-
Notifications
You must be signed in to change notification settings - Fork 915
Update LSP4J to 0.24.0 and Xtend/Xtext to 2.32.0 #9104
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 LSP4J to 0.24.0 and Xtend/Xtext to 2.32.0 #9104
Conversation
Update dependencies: bump LSP4J to 0.24.0 and Xtext to 2.32.0, replace deprecated API usages. Further cleaned up some Tests to better conform to changes in newer versions of LSP4J.
|
Just curious, what happens when all tests pass, I believe a review process happens? |
|
@dasTanmoy0096 the process is less formal. Commits need at least one NetBeans committer to merge. It is a judgement call to merge. The unittests give a first indicator, review of the changes and potentially testing give the second factor. The process is not necessarily limited to a single reviewer. Depending on the scope of the changes secondary opinions can be requested or other people chime in based on their area of interest. Please indicate how you tested the changes. |
matthiasblaesing
left a comment
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.
The changes make sense to me. The typescript integration still works, so I assume the client changes are fine.
@MartinBalin could you please have a look at the lsp.server changes? These look sane to me, but I don't use the LSP, so someone from that area would be good to have a look.
|
I've put the dependencies into a pom to check the dependency tree for surprises and it should be fine I think. gson is in range and guava is is fine too. (although it wouldn't hurt to update them separately at some point; edit: #9114) Details <dependencies>
<dependency>
<groupId>org.eclipse.lsp4j</groupId>
<artifactId>org.eclipse.lsp4j</artifactId>
<version>0.24.0</version>
</dependency>
<dependency>
<groupId>org.eclipse.lsp4j</groupId>
<artifactId>org.eclipse.lsp4j.generator</artifactId>
<version>0.24.0</version>
</dependency>
<dependency>
<groupId>org.eclipse.lsp4j</groupId>
<artifactId>org.eclipse.lsp4j.jsonrpc</artifactId>
<version>0.24.0</version>
</dependency>
<dependency>
<groupId>org.eclipse.lsp4j</groupId>
<artifactId>org.eclipse.lsp4j.debug</artifactId>
<version>0.24.0</version>
</dependency>
<dependency>
<groupId>org.eclipse.lsp4j</groupId>
<artifactId>org.eclipse.lsp4j.jsonrpc.debug</artifactId>
<version>0.24.0</version>
</dependency>
<dependency>
<groupId>org.eclipse.xtend</groupId>
<artifactId>org.eclipse.xtend.lib</artifactId>
<version>2.32.0</version>
</dependency>
<dependency>
<groupId>org.eclipse.xtend</groupId>
<artifactId>org.eclipse.xtend.lib.macro</artifactId>
<version>2.32.0</version>
</dependency>
<dependency>
<groupId>org.eclipse.xtext</groupId>
<artifactId>org.eclipse.xtext.xbase.lib</artifactId>
<version>2.32.0</version>
</dependency>
</dependencies> |
|
Hi, yes, looking into LSP, thank you for pinging me. |
sdedic
left a comment
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.
LGTM, looking at the lsp server part. I've tested briefly vscode apache nbls extension built on top of this branch as well.
matthiasblaesing
left a comment
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.
With both server and client covered, lets get this in.
|
@dasTanmoy0096 thanks for work |
Update dependencies: bump LSP4J to 0.24.0 and Xtext to 2.32.0, replace deprecated API usages.
Further cleaned up some Tests to better conform to changes in newer versions of LSP4J.
Since #9098 kept failing due to incorrect email in commit header, I closed the previous PR and raised this one.