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

prepare NB 19 release cycle for JDK 11. #5852

Merged
merged 1 commit into from
Apr 28, 2023

Conversation

mbien
Copy link
Member

@mbien mbien commented Apr 19, 2023

  • disable CV tests on JDK 8
  • move some more jobs to JDK 11

this configuration would allow us to move new modules like rust/go/hcl to JDK 11 and to upgrade exiting modules, like the maven-indexer #4999.

new NB release policy

 - disable CV tests on JDK 8
 - move some more jobs to JDK 11

this configuration would allow us to move new modules like rust/go/hcl
to JDK 11 and to upgrade exiting modules, like the maven-indexer apache#4999.
@mbien mbien added CI Continuous Integration ci:all-tests [ci] enable all tests labels Apr 19, 2023
@mbien mbien added this to the NB19 milestone Apr 19, 2023
@neilcsmith-net
Copy link
Member

todo: wiki link to new NB release policy

Sorry for the delay with that - https://cwiki.apache.org/confluence/display/NETBEANS/Minimum+JDK+build+and+run+policy

@mbien mbien marked this pull request as ready for review April 20, 2023 17:55
@jlahoda
Copy link
Contributor

jlahoda commented Apr 27, 2023

FWIW, there should be a PR at some point (not necessarily here) that would provide user friendly failure messages when the build or binary is used on an unsupported JDK.

@neilcsmith-net
Copy link
Member

FWIW, there should be a PR at some point (not necessarily here) that would provide user friendly failure messages when the build or binary is used on an unsupported JDK.

Well, we've really needed that for the IDE since NetBeans 13!

Personally, I would now look to bump the bytecode level in non-API bits of the runtime container. eg. check and fail, and update the error messages, at https://github.com/apache/netbeans/blob/master/platform/o.n.bootstrap/src/org/netbeans/Main.java#L43

If as well as checking for JDK, we updated the bytecode level, it would also allow for removing some reflection in #3386 ?

java: [ '8' ]
java: [ '11' ]
Copy link
Member Author

Choose a reason for hiding this comment

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

@dbalek @sdedic this would move the NBVSCode ext and the LSP job to JDK 11 since those jobs load all other modules. If the VSCode Ext has different requirements we would have to change the test setup there.

But from past conversations I believe to remember that JDK 11 was no problem there, right?

@neilcsmith-net
Copy link
Member

Follow up to brief comment on the Markdown PR.

Once we're ready for this, we should also add a new label for PRs that update the Java requirements for any module - "non-optional modules are free to adopt the minimum JDK as they need to." should be part of the review process. min-JDK-change ??

Copy link
Contributor

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

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

Looks sane to me.

@mbien
Copy link
Member Author

mbien commented Apr 28, 2023

Once we're ready for this, we should also add a new label for PRs that update the Java requirements for any module - "non-optional modules are free to adopt the minimum JDK as they need to." should be part of the review process. min-JDK-change ??

@neilcsmith-net we have Upgrade Library Library (Dependency) Upgrade , how about just Upgrade JDK Upgrade to the JDK requirements of a module. ? We could explain it in the label tooltip.

FWIW, there should be a PR at some point (not necessarily here) that would provide user friendly failure messages when the build or binary is used on an unsupported JDK.

agreed

@mbien
Copy link
Member Author

mbien commented Apr 28, 2023

I got a thumbs up #5852 (comment) and one approval. So I interpret this as go-ahead -> merging.

@mbien mbien merged commit 613087a into apache:master Apr 28, 2023
37 checks passed
@mbien
Copy link
Member Author

mbien commented Apr 30, 2023

@neilcsmith-net are you ok with the name of the new label?

@neilcsmith-net
Copy link
Member

@mbien looks good. I don't really mind the name, as long as we have one and all know what it means! 😆

@mbien
Copy link
Member Author

mbien commented May 7, 2023

@vieiro @lkishalmi I remember from reviews that you both tried to use language level 11 before for rust and similar independent modules. This is now possible.

simply mark a PR with Upgrade JDK Upgrade to the JDK requirements of a module. so that we can track it and bump the source/target property for the module.

@vieiro
Copy link
Contributor

vieiro commented May 7, 2023

@mbien I'm with with language level 8 at the moment. Good to know we can mark things for 11, though.

@mbien
Copy link
Member Author

mbien commented Jun 8, 2023

see #6054 regarding JDK requirements warning UI

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 CI Continuous Integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants