-
Notifications
You must be signed in to change notification settings - Fork 917
Backport VSNetBeans fixes to 12.2 branch. #2626
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
Backport VSNetBeans fixes to 12.2 branch. #2626
Conversation
8c52e9a to
a72e011
Compare
|
Just as a general hint: Taking git commits and applying them to other branches is the domain of "cherry-pick". So given the above and only looking at the first 5 entries of the list, this would be the approach I suggest: The benefit is, that git will take care of stopping at the right points if manual adjustments are necessary and can take history into account. In general the approach taken looks ok, but I can see situation where this breaks hard (binaries, rejects). |
|
I assume The spec version increments look suspect to me. eg. would expect |
|
Thanks for the New Year's Eve reviews! I'll redo the commits using |
…LSP client and Java LSP server.
…neccessary prefix.
…to execute integration tests for VSNetBeans
…nning java.compile.workspace command
…ons's CompletableFuture
…or the Java LSP server.
…o a MIME type of JavaScript.
…ed!' by ignoring StdErr when looking for match
…est in VSNetBeans. It started to fail for unknown reason after integration.
a72e011 to
84d8a49
Compare
|
I tried to use
As such I decided to just replay the original script and then manually adjust dependencies by increasing the 3rd digit. |
|
Ähm - you are actively deleteing rejects: find . | grep rej$ | xargs rm I have some serious doubts, that that is a good idea. And the question is: If there are merge conflicts: Why are they there? Where are the differing commits? Yes the merge commits are a problem, because cherry-pick applies the changes, but merge pulls two branches together, which branch should cherry-pick as a basis for the merge? This is the reason I like to cleanup after doing a messy PR and compress the 17 intermediate commits into one or two meaning full commits, that makes it much easier to deal with history changes like this. But you can work around that by specifying the commits from the branches you merged, that is already the case here: merged For the huge number of conflicts, I suspect the problem is, that the order of the commits is reversed. The first entry in your list is the most recent, while cherry-pick needs the "right" order or commits. |
|
Re. order: My script is using Re. deleting manual inspection indicates that the conflict is only in versioning. This PR's last commit (84d8a49) sets the version to |
jlahoda
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.
I skimmed through the list of commits - seems sensible to me.
Here is my attempt to backport fixes made in
mastersince 12.2 ontorelease122branch. I have executed following script:to generate all commits that touch
java/java.lsp.serverfolder. Then I commented few merges out and got:then I applied the changes using this script:
after doing all of that the content of
java/java.lsp.serveris quite similar to its state inmasterbranch (except version information):netbeans$ git diff -r origin/master java/java.lsp.server/ | grep ^diff diff --git a/java/java.lsp.server/nbproject/org-netbeans-modules-java-lsp-server.sig b/java/java.lsp.server/nbproject/org-netbeans-modules-java-lsp-server.sig diff --git a/java/java.lsp.server/nbproject/project.properties b/java/java.lsp.server/nbproject/project.properties diff --git a/java/java.lsp.server/vscode/package.json b/java/java.lsp.server/vscode/package.jsonLet's see if everything builds OK. Guys, please verify your changes are in. @lkishalmi please confirm it is OK to merge into
release122- I have seen it was OK when producing12.0-u1, so hopefully it is OK to continue pushing torelease122as well.