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

Temporarily downgrade gradle module language level #7367

Merged

Conversation

mbien
Copy link
Member

@mbien mbien commented May 8, 2024

  • resolves a "Project Problems" regression
  • temporary until proper fix is implemented

fixes #7365 ? untested

 - resolves a "Project Problems" regression
 - temporary until proper fix is implemented
@mbien mbien added Gradle [ci] enable "build tools" tests ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels May 8, 2024
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.

I had the same though - you were faster 😁

On the switches I would not throw at runtime, but other than that I reached pretty similar conclusions.

switch(this) {
case GROOVY: return Bundle.LBL_DSL_GROOVY();
case KOTLIN: return Bundle.LBL_DSL_KOTLIN();
default: throw new IllegalStateException("update switch");
Copy link
Contributor

Choose a reason for hiding this comment

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

I would not throw from toString - if this is user visible the name might look strange, but still conveys the meaning.

Suggested change
default: throw new IllegalStateException("update switch");
default: return name();

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah but I am sure @lkishalmi will fix this properly during NB 23, so we can't really arrive at a situation where enums are added (which are btw a few lines above the toString()) without anyone noticing it.

case SPOCK: return Bundle.LBL_TFW_SPOCK();
case TESTNG: return Bundle.LBL_TFW_TESTNG();
case XCTEST: return Bundle.LBL_TFW_XCTEST();
default: throw new IllegalStateException("update switch");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
default: throw new IllegalStateException("update switch");
default: return name();

@mbien mbien added this to the NB22 milestone May 8, 2024
@mbien mbien linked an issue May 8, 2024 that may be closed by this pull request
@mbien mbien marked this pull request as ready for review May 8, 2024 19:07
@mbien mbien requested a review from lkishalmi May 8, 2024 19:07
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.

Looks good to me!

@ebarboni
Copy link
Contributor

do we let the exception throw ?

@lkishalmi
Copy link
Contributor

do we let the exception throw ?

Well, that default part should not happen. The new switch checks for completeness. Reverting back to the old syntax and trying to be complete would add a default case.

I'm going to split the Gradle module for NB23, as it seems we need some parts kept on Java 8 level for a while, while the majority of code would be fine on Java 17.

@neilcsmith-net
Copy link
Member

Yes, there's a synthesized default that throws an exception in the current code - no difference IMO.

@lkishalmi yes, it also makes sense to isolate any class that might be sent over the wire into the daemon process anyway, source level or not?

@ebarboni ebarboni merged commit 305db5d into apache:delivery May 14, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) Gradle [ci] enable "build tools" tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[REGRESSION] Gradle "Project Problems"
5 participants