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

Generate projects for Java 11 by default #8151

Merged
merged 3 commits into from
Mar 26, 2020

Conversation

gsmet
Copy link
Member

@gsmet gsmet commented Mar 25, 2020

@gastaldi I created the PR early so that you can have a look too.

I will try generating projects now :).

@boring-cyborg boring-cyborg bot added the area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins label Mar 25, 2020
Copy link
Contributor

@gastaldi gastaldi left a comment

Choose a reason for hiding this comment

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

LGTM

@gastaldi gastaldi added this to the 1.4.0 milestone Mar 25, 2020
@gsmet
Copy link
Member Author

gsmet commented Mar 25, 2020

OK, I tested all the project creation combinations, it was fun \o/.

@gsmet
Copy link
Member Author

gsmet commented Mar 25, 2020

@patriot1burke should we also make this change for your Amazon Lambda archetypes? Or do you want to stick to 8 for now?

@dmlloyd dmlloyd mentioned this pull request Mar 25, 2020
18 tasks
@gastaldi
Copy link
Contributor

You need to remove the Quarkus CI / JDK 8 JVM Tests from the GH Actions descriptor too

By default, it is based on the Java version used to create the project.
It can be overridden in CreateProject though if you really want to
enforce it in your tools.

I haven't exposed it for now as we will probably remove it or tweak it
later.

Note that even if you enforce it, it will still use either 8 or 11 for
now. We might want to further enhance that later.
@boring-cyborg boring-cyborg bot added the area/platform Issues related to definition and interaction with Quarkus Platform label Mar 25, 2020
@gsmet
Copy link
Member Author

gsmet commented Mar 25, 2020

@gastaldi I don't want to do that. We want to support 8 still. But I think I pushed an appropriate work around.

@@ -134,6 +147,15 @@ public boolean doCreateProject(final Map<String, Object> context) throws IOExcep
}

public QuarkusCommandOutcome execute() throws QuarkusCommandException {
// Define the Java version to use determined from the one specified or the one creating the project
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment isn't really helpful, just saying :)

@patriot1burke
Copy link
Contributor

patriot1burke commented Mar 25, 2020 via email

Matcher matcher = JAVA_VERSION_PATTERN
.matcher(this.javaTarget != null ? this.javaTarget : System.getProperty("java.version", ""));
if (matcher.matches() && Integer.parseInt(matcher.group(1)) < 11) {
invocation.setProperty(JAVA_TARGET, invocation.getBuildTool() == BuildTool.MAVEN ? "1.8" : "1_8");
Copy link
Member

Choose a reason for hiding this comment

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

why not have one "java_target" and another "java_underscore_target" so the same property isn't to be treated differently ?

Copy link
Member Author

Choose a reason for hiding this comment

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

That was the other option I pondered. I decided to go with this one.

@@ -107,7 +107,7 @@
</executions>
<configuration>
<javaParameters>true</javaParameters>
<jvmTarget>1.8</jvmTarget>
<jvmTarget>${java_target}</jvmTarget>
Copy link
Member

Choose a reason for hiding this comment

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

so if grok this right this change is a change in the "api" of the code generation that means the templates now require java_target to be set which will not be the case if you are using 1.3.0 plugin (@aloubyansky correct me if i'm wrong on this here).

So 1.3.0 plugin will pick up latest 1.3.x platform and when generation happens it will fail.

To remedy this the various .ftl files should check if java_target is set.
if my freemarker syntax memory is right its something like ${java_target!"11"}

Copy link
Member

Choose a reason for hiding this comment

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

so if grok this right this change is a change in the "api" of the code generation that means the templates now require java_target to be set which will not be the case if you are using 1.3.0 plugin (@aloubyansky correct me if i'm wrong on this here).

That is correct.

Copy link
Member Author

Choose a reason for hiding this comment

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

I suppose we should choose 1.8 as the default to have a consistent behavior. I'll go do that and test.

@maxandersen
Copy link
Member

I approve despite I feel dirty doing so :)

@gsmet
Copy link
Member Author

gsmet commented Mar 26, 2020

@maxandersen yeah... and you're not the one who had to write this :).

@gsmet gsmet merged commit 4b5ec65 into quarkusio:master Mar 26, 2020
@gsmet gsmet modified the milestones: 1.4.0.CR1, 1.3.2.Final Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/platform Issues related to definition and interaction with Quarkus Platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants