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

Fixes quarkus-platform native TS config #15929

Merged
merged 1 commit into from
Mar 23, 2021

Conversation

aloubyansky
Copy link
Member

@aloubyansky aloubyansky commented Mar 22, 2021

quarkus-platform TS config was using native-image goal of the quarkus-maven-plugin that was recently removed. native-image allowed to configure an alternative artifact as the app artifact in place of the current project. Which is how imported tests are run. With the native-image goal removed there is no way to configure the alternative app artifact.
This PR adds appArtifact parameter to the QuarkusBootstrapMojo that is equivalent to the removed from the NativeImageMojo one.

@quarkus-bot quarkus-bot bot added area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/maven labels Mar 22, 2021
@aloubyansky aloubyansky requested a review from gsmet March 22, 2021 14:53
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Let's clarify my question before merging.

@@ -195,6 +171,96 @@ protected CuratedApplication curateApplication(QuarkusBootstrapMojo mojo) throws
}
}

protected AppArtifact managingProject(QuarkusBootstrapMojo mojo) {
if (mojo.appArtifactCoords() == null) {
return null;
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 it's an external artifact, you return null here? What's bothering me a bit is that it wasn't set before so I'm not sure to understand why it's now set in the case that was already existing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. When the appArtifact is configured, the current project is used as the environment for the build and tests. That's the meaning behind the managingProject. It's either null or the current project. Setting it to null is equivalent to not setting it at all.

@gsmet gsmet merged commit c45b1a6 into quarkusio:main Mar 23, 2021
@quarkus-bot quarkus-bot bot added this to the 1.14 - main milestone Mar 23, 2021
@gsmet
Copy link
Member

gsmet commented Mar 23, 2021

I think it's best not backporting it for now. Let's fix this for 1.14, it has been broken for a while anyway.

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/maven
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants