-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Bootstrap API refactoring #20500
Bootstrap API refactoring #20500
Conversation
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 9be6a31
Full information is available in the Build summary check run. Failures⚙️ MicroProfile TCKs Tests #- Failing: tcks/microprofile-fault-tolerance
📦 tcks/microprofile-fault-tolerance✖
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! That will definitely ease application model usage. I left few comments.
|
||
} | ||
|
||
@Override | ||
public Path resolve(AppArtifact appArtifact) throws AppModelResolverException { | ||
return resolveArtifact(appArtifact).getPaths().getSinglePath(); | ||
public io.quarkus.maven.dependency.ResolvedDependency resolve(ArtifactCoords appArtifact) throws AppModelResolverException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think those imports can be global in the file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, now the class/interface names don't conflict with the Gradle API, adding the imports instead.
@@ -249,7 +238,8 @@ private File getLastFile(FileCollection fileCollection) { | |||
* @return the source sets associated with the current project. | |||
*/ | |||
private SourceSetContainer getSourceSets() { | |||
return project.getConvention().getPlugin(JavaPluginConvention.class).getSourceSets(); | |||
final JavaPluginExtension javaExtension = project.getExtensions().findByType(JavaPluginExtension.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method has been introduce in gradle 7.1, as they also deprecate convention in that version, I think we can stick with convention for now as gradle 7.1 is quite recent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, changing back to the convention, thanks!
|
||
default Collection<WorkspaceModule> getWorkspaceModules() { | ||
final List<WorkspaceModule> modules = new ArrayList<>(); | ||
//ProjectModule module = getApplicationModule(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we keep this commented block of code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally I left it as a reminder to myself to review why we don't add the application module to the complete list of locally available dependencies. But it's not a big deal. I'll remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally I left it as a reminder to myself to review why we don't add the application module to the complete list of locally available dependencies. But it's not a big deal. I'll remove it.
AppArtifact appArtifact = new AppArtifact("dev.jbang.user", "quarkus", null, "jar", "999-SNAPSHOT"); | ||
appArtifact.setPath(appClasses); | ||
final ResolvedArtifactDependency appArtifact = new ResolvedArtifactDependency("dev.jbang.user", "quarkus", null, | ||
"jar", "999-SNAPSHOT", appClasses); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was present before your modification but should we get rid of the hard coded version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's not important what this version actually is. I'll leave it for now.
9be6a31
to
964727a
Compare
Serialize the ApplicationModel in DevMojo to avoid re-resolving it in the dev process
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 964727a
Failures⚙️ Devtools Tests - JDK 11 #- Failing: integration-tests/devtools
📦 integration-tests/devtools✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ⚙️ Devtools Tests - JDK 11 Windows #- Failing: integration-tests/devtools
📦 integration-tests/devtools✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ⚙️ Gradle Tests - JDK 11 #- Failing: integration-tests/gradle
📦 integration-tests/gradle✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ⚙️ Gradle Tests - JDK 11 Windows #- Failing: integration-tests/gradle
📦 integration-tests/gradle✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ✖ ⚙️ JVM Tests - JDK 11 Windows #- Failing: devtools/cli devtools/gradle/gradle-application-plugin
📦 devtools/cli✖ ✖ ✖ ✖ 📦 devtools/gradle/gradle-application-plugin✖ ⚙️ Native Tests - Misc4 #- Failing: integration-tests/gradle
📦 integration-tests/gradle✖ ✖ |
964727a
to
8073bad
Compare
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 03b38ad
Failures⚙️ Initial JDK 11 Build #- Failing: devtools/cli integration-tests/avro-reload integration-tests/grpc-health and 11 more
📦 devtools/cli✖ 📦 integration-tests/avro-reload✖ 📦 integration-tests/grpc-health✖ 📦 integration-tests/grpc-hibernate✖ 📦 integration-tests/grpc-hibernate-reactive✖ 📦 integration-tests/grpc-interceptors✖ 📦 integration-tests/grpc-mutual-auth✖ 📦 integration-tests/grpc-plain-text-gzip✖ 📦 integration-tests/grpc-plain-text-mutiny✖ 📦 integration-tests/grpc-proto-v2✖ 📦 integration-tests/grpc-streaming✖ 📦 integration-tests/grpc-tls✖ 📦 integration-tests/kafka-avro✖ 📦 integration-tests/kafka-avro-apicurio2✖ |
03b38ad
to
65ae5bc
Compare
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 65ae5bc
Full information is available in the Build summary check run. Failures⚙️ JVM Tests - JDK 11 #- Failing: integration-tests/kotlin integration-tests/scala
📦 integration-tests/kotlin✖
📦 integration-tests/scala✖
⚙️ JVM Tests - JDK 11 Windows #- Failing: integration-tests/kotlin integration-tests/scala
📦 integration-tests/kotlin✖
📦 integration-tests/scala✖
⚙️ JVM Tests - JDK 17 #- Failing: integration-tests/kotlin integration-tests/scala
📦 integration-tests/kotlin✖
📦 integration-tests/scala✖
⚙️ Maven Tests - JDK 11 #- Failing: integration-tests/maven
📦 integration-tests/maven✖
✖
✖
✖
✖
✖
✖
✖
✖
✖
|
65ae5bc
to
0988194
Compare
… the app in the dev mode; Make DevMojo re-use the app model resolved as part of the generate-code goal
0988194
to
870cfb7
Compare
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 870cfb7
Full information is available in the Build summary check run. Failures⚙️ JVM Tests - JDK 17 #- Failing: extensions/hibernate-orm/deployment
! Skipped: docs extensions/hibernate-envers/deployment extensions/hibernate-reactive/deployment and 84 more 📦 extensions/hibernate-orm/deployment✖
|
@stuartwdouglas what do you think about getting this in 2.4? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good.
Failing Jobs - Building 870cfb7
Full information is available in the Build summary check run. Failures⚙️ Gradle Tests - JDK 11 Windows #- Failing: integration-tests/gradle
📦 integration-tests/gradle✖
|
This PR introduces a common Quarkus application model and workspace API that are supposed to be used for all the supported build systems (e.g. Maven, Gradle, JBang, etc).
I still plan to evolve the representation of the workspace module sources but I was thinking I'd open this PR now for review, since it's already a huge change.
It also reduces the number of workspace and application model resolutions for the dev mode. I am looking into getting it down to a single app model initialization in a different branch.