Adapt LSP4E to LSP4J vers. 1.0.0#1421
Conversation
travkin79
force-pushed
the
feature/lsp4j-1
branch
from
47ec245 to
fdcf0fd
Compare
|
We should hold merging this back until LSP4J 1.0 is actually released otherwise we are blocked from cutting LSP4E releases ourselves from now on. |
|
Hi @sebthom,
I expected that. Would you prefer to change this PR's status to draft? |
|
I think that is appropriate for now. |
jonahgraham
left a comment
jonahgraham
left a comment
There was a problem hiding this comment.
This LGTM - I have done code inspection, but not run it up in LSP4E.
travkin79
force-pushed
the
feature/lsp4j-1
branch
2 times, most recently
from
d227f85 to
ee645ca
Compare
travkin79
force-pushed
the
feature/lsp4j-1
branch
2 times, most recently
from
e584f6b to
9239d88
Compare
target-platforms/target-platform-latest/target-platform-latest.target
Outdated
Show resolved
Hide resolved
travkin79
force-pushed
the
feature/lsp4j-1
branch
from
bd4fabe to
1879822
Compare
| org.eclipse.lsp4j.jsonrpc;bundle-version="[0.24.0,0.25.0)", | ||
| org.eclipse.lsp4j.jsonrpc.debug;bundle-version="[0.24.0,0.25.0)", | ||
| org.eclipse.lsp4j.debug;bundle-version="[0.24.0,0.25.0)", | ||
| org.eclipse.lsp4j.jsonrpc;bundle-version="[1.0.0,2.0.0)", |
There was a problem hiding this comment.
should all these not use 1.1.0 instead of 2.0.0?
There was a problem hiding this comment.
No, it is intentionally set to 1.0.0. There was no LSP4E release yet based on the latest LSP4J release vers. 1.0.0 and LSP4J vers. 1.1.0 is still under development. see #1421 (comment)
Should LSP4E not always depend on a released LSP4J version?
There was a problem hiding this comment.
I did not mean to change 1.0.0, I meant change 2.0.0 to 1.1.0, which would be the next minor version bump, equivalent to what we have been doing.
There was a problem hiding this comment.
Oh, I should have read your comment more properly. Sorry for that.
But I'm still not sure if an upper range limit of 1.1.0 instead of 2.0.0 is what we want. Starting with LSP4J 1.0.0, semantic versioning is used. Doesn't that mean that increasing the minor version of LSP4J would not introduce any breaking change and we can leave the upper range limit being 2.0.0?
There was a problem hiding this comment.
I see your point. Let's go for this. I have rerun one failing build and if it passes, I will merge it.
There was a problem hiding this comment.
@travkin79 , actually I cannot merge. Can you do the rebase?
There was a problem hiding this comment.
Its already merged, but here is a +1 review as requested.
Doesn't that mean that increasing the minor version of LSP4J would not introduce any breaking change and we can leave the upper range limit being 2.0.0?
I hope so! Assuming LSP4J doesn't screw up.
It may not mean any less updates needed to version ranges, but at least those updates will be semantic version compliant. i.e the next version LSP4J releases may well be 2.0.0, it is still TBD depending on what patches/improvements the project gets.
| org.eclipse.ui.navigator;bundle-version="3.6.100", | ||
| org.eclipse.lsp4j;bundle-version="[0.24.0,0.25.0)", | ||
| org.eclipse.lsp4j.jsonrpc;bundle-version="[0.24.0,0.25.0)", | ||
| org.eclipse.lsp4j;bundle-version="[1.0.0,2.0.0)", |
rubenporras
left a comment
rubenporras
left a comment
There was a problem hiding this comment.
could you please adapt the version ranges?
For the sake of simplicity, for now, we only use the left argument in Either tuples, i.e. the types that we used in earlier versions.
Thanks @jonahgraham Co-authored-by: Jonah Graham <jonah@kichwacoders.com>
travkin79
force-pushed
the
feature/lsp4j-1
branch
from
1879822 to
78758bd
Compare
4 participants
Adapt LSP4E to LSP4J API changes as suggested by @jonahgraham. Other PRs requiring LSP4J vers. 1.0.0 like PR #1149 will base upon changes from this PR.