Skip to content
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

Recognize nbjrt: protocol and locate JDK9+ src.zip #5157

Merged
merged 3 commits into from
Dec 30, 2022

Conversation

jtulach
Copy link
Contributor

@jtulach jtulach commented Dec 28, 2022

The PlatformForSourceBinaryQuery class contains support for so called "unregistered platforms". However it seems that the code hasn't been updated to JDK9+ layout. This PR does that. The previously existing test (for JDK8 layout) has been copied and modified to mimic the layout of JDK9+.

@jtulach jtulach self-assigned this Dec 28, 2022
Copy link
Contributor

@lkishalmi lkishalmi left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Contributor

@jlahoda jlahoda left a comment

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 added Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) ci:all-tests [ci] enable all tests labels Dec 29, 2022
Copy link
Member

@mbien mbien left a comment

Choose a reason for hiding this comment

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

please make sure this PR builds again before merge. It wasn't labeled. Most of the java tests did not run. (also for the reviewers, please label PRs before approval :))

added ci:all-tests [ci] enable all tests just to be sure.

Comment on lines 100 to 104
static SourceForBinaryQueryImplementation2.Result searchUnregisteredPlatform(String binaryRootS) {
String srcZipS = null;
String srcZipIn = null;
if (binaryRootS.startsWith(JAR_FILE)) {
if (binaryRootS.endsWith(RTJAR_PATH)) {
Copy link
Member

Choose a reason for hiding this comment

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

would be good to use standard indentation settings for new code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would help if NetBeans apisupport projects specified the proper indentation settings. Otherwise the source gets formatted by my global defaults (which are set to Google Java standards, not Sun/Oracle now).

Reformatted in 5b0d432

Copy link
Member

Choose a reason for hiding this comment

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

awesome thanks.

Yeah its probably a good idea to copy a few selected IDE defaults to the project settings. Its probably not worth it though if it has to be set up for every single module, would be too much noise for little gain IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A project type can provide some default AuxiliarySettings - java.mx.project module is doing that to influence the Eclipse formatter (used for mx projects).

Copy link
Contributor

Choose a reason for hiding this comment

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

NetBeans Formatter needs one more level of indirection, as it currently uses the project's auxiliary properties directly. If I had time...

@@ -82,6 +80,23 @@ public void testUnregisteredPlatform() throws Exception {
assertNull(result);
}

public void testUnregisteredJDK11Platform() throws Exception {
File wd = getWorkDir();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jtulach jtulach merged commit 1e45548 into apache:master Dec 30, 2022
@neilcsmith-net neilcsmith-net added this to the NB17 milestone Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:all-tests [ci] enable all tests Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants