Skip to content

nb-javac checks cleanup and dialog removal. - #3396

Merged
mbien merged 4 commits into
apache:masterfrom
mbien:nb-javac-checks
Jan 8, 2022
Merged

nb-javac checks cleanup and dialog removal.#3396
mbien merged 4 commits into
apache:masterfrom
mbien:nb-javac-checks

Conversation

@mbien

@mbien mbien commented Jan 1, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

post nb-javac 17 on-by-default refactoring

@mbien mbien added the do not merge Don't merge this PR, it is not ready or just demonstration purposes. label Jan 1, 2022
jlahoda
jlahoda approved these changes Jan 1, 2022
View reviewed changes

@jlahoda jlahoda left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great, this will be helpful! I've left a few comments inline.

Comment thread java/java.source.base/src/org/netbeans/modules/java/source/NoJavacHelper.java Outdated
Comment thread java/java.source.base/src/org/netbeans/modules/java/source/NoJavacHelper.java
Comment thread java/java.source.base/src/org/netbeans/modules/java/source/indexing/JavaCustomIndexer.java Outdated
Comment thread ...e.base/test/unit/src/org/netbeans/modules/java/source/indexing/VanillaCompileWorkerTest.java Outdated
@mbien

mbien commented Jan 1, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

@jlahoda thats too early for a review - i hacked this quickly together to get rid of the startup dialog :) But thanks I will follow your suggestions of course. More commits will follow which remove workarounds I added in past PRs for example. (travis is inactive?)

@mbien
mbien force-pushed the nb-javac-checks branch 3 times, most recently from 6a21c7f to b75771c Compare January 3, 2022 10:55
@mbien
mbien marked this pull request as ready for review January 3, 2022 11:07
@mbien mbien added the Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) label Jan 3, 2022
@mbien mbien removed the do not merge Don't merge this PR, it is not ready or just demonstration purposes. label Jan 3, 2022
@mbien

mbien commented Jan 3, 2022

Copy link
Copy Markdown
Member Author

@jlahoda incorporated your review remarks into the first commit + added 3 more commits.

mbien
mbien commented Jan 3, 2022
View reviewed changes
Comment thread java/spi.java.hints/src/org/netbeans/modules/java/hints/spiimpl/Utilities.java Outdated
Comment on lines 1455 to 1456
/* TODO: unused? called a method which is only available from 8-11.
protected JCCase switchBlockStatementGroupImpl() throws Throwable {

@mbien mbien Jan 3, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

i commented it out, but I think we can remove this, right?

@jlahoda jlahoda Jan 5, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, I think so.

@mbien mbien Jan 5, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

All tests green with the requested changes integrated.

Lets try to remove the ByteBuddy interceptor which delegates to one of the two methods.

@mbien mbien Jan 5, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

interceptor/reflection removal is squashed with the jackpot trees commit

@mbien

mbien commented Jan 3, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

travis jobs 13 and 16 are failing.

@mbien mbien added the do not merge Don't merge this PR, it is not ready or just demonstration purposes. label Jan 3, 2022
@JaroslavTulach
JaroslavTulach requested a review from dbalek January 4, 2022 14:27
@mbien mbien added the Code cleanup Label for cleanup done on the Netbeans IDE label Jan 4, 2022
jlahoda
jlahoda approved these changes Jan 5, 2022
View reviewed changes
Comment thread java/java.source.base/src/org/netbeans/modules/java/source/indexing/JavaCustomIndexer.java
Comment thread ...e.base/test/unit/src/org/netbeans/modules/java/source/indexing/VanillaCompileWorkerTest.java Outdated
Comment thread java/spi.java.hints/src/org/netbeans/modules/java/hints/spiimpl/Utilities.java Outdated
Comment on lines 1455 to 1456
/* TODO: unused? called a method which is only available from 8-11.
protected JCCase switchBlockStatementGroupImpl() throws Throwable {

@jlahoda jlahoda Jan 5, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, I think so.

@mbien
mbien force-pushed the nb-javac-checks branch 3 times, most recently from 1186492 to 737837c Compare January 6, 2022 01:50
mbien added 2 commits January 6, 2022 03:40
@mbien
trees nb-javac 17 cleanup after apache#3283.
7414fb4 
 - removed ByteBuddy interceptors and dead code path
 - less reflection
@mbien
SourceVersion related cleanup.
837f2e6
@mbien mbien removed the do not merge Don't merge this PR, it is not ready or just demonstration purposes. label Jan 6, 2022
@mbien

mbien commented Jan 6, 2022

Copy link
Copy Markdown
Member Author

everything green now. Had to brute force through job 13 though

org.netbeans.modules.java.lsp.server.protocol.ServerTest
    failed: testCancelProgressHandle

restart job
org.netbeans.modules.gradle.java.classpath.ClassPathProviderImplTest
    failed: testCompilePreTrusted

restart job
org.netbeans.modules.java.lsp.server.protocol.ServerTest
    failed: testCancelProgressHandle

restart job
green

@mbien

mbien commented Jan 7, 2022

Copy link
Copy Markdown
Member Author

@jlahoda @JaroslavTulach @matthiasblaesing ok to merge?

@JaroslavTulach

JaroslavTulach commented Jan 8, 2022

Copy link
Copy Markdown

everything green now. Had to brute force through job 13 though

org.netbeans.modules.java.lsp.server.protocol.ServerTest
    failed: testCancelProgressHandle

Sorry for that. Feel free to mark the test as @RandomlyFails. The maintainers of the ServerTest shall suffer, not people working on the rest of NetBeans!

jlahoda
jlahoda approved these changes Jan 8, 2022
View reviewed changes

@jlahoda jlahoda left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me.

@mbien
mbien merged commit 4a148ec into apache:master Jan 8, 2022
@neilcsmith-net neilcsmith-net added this to the NB13 milestone Jan 26, 2022
@mbien mbien mentioned this pull request Nov 29, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@matthiasblaesing matthiasblaesing Awaiting requested review from matthiasblaesing
@dbalek dbalek Awaiting requested review from dbalek
2 more reviewers
@jlahoda jlahoda jlahoda approved these changes
@JaroslavTulach JaroslavTulach JaroslavTulach approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Code cleanup Label for cleanup done on the Netbeans IDE Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form)

Projects

None yet

Milestone

NB13

Development

Successfully merging this pull request may close these issues.

4 participants

@mbien @JaroslavTulach @jlahoda @neilcsmith-net