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.

Sign up for GitHub

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

Jump to bottom

Refactor and Introduce ExtensionManager in CodeGen #9787

Merged
merged 1 commit into from
Jun 10, 2020
Merged

Refactor and Introduce ExtensionManager in CodeGen #9787

merged 1 commit into from
Jun 10, 2020

Conversation

ia3andy
Copy link
Contributor

@ia3andy ia3andy commented Jun 4, 2020 •
edited
Loading

This is part of #8178

  • Improved packages names and content
  • Improved general readability and consistency
  • Added ExtensionManager to define a high level way of managing (read/write) extensions in any QuarkusProject
  • Removed most unsafe Gradle operations which were outside of the Gradle plugin (we need to figure out a way to improve the "generic" gradle support to make it compatible again..)
  • Removed existing project support with the create command (throws an error) -> Migrate project command in Quarkus tooling #9875
  • Removed compatibility with project without the Quarkus platform bom defined

@boring-cyborg boring-cyborg bot added area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/gradle Gradle area/maven area/platform Issues related to definition and interaction with Quarkus Platform labels Jun 4, 2020
@ia3andy ia3andy force-pushed the introduce-quarkus-project branch from 69e427b to 4e3abc3 Compare June 4, 2020 12:52
@ia3andy ia3andy mentioned this pull request Jun 4, 2020
Merged
@ia3andy ia3andy force-pushed the introduce-quarkus-project branch from 4e3abc3 to 56e060f Compare June 4, 2020 12:58
@ia3andy
Copy link
Contributor Author

ia3andy commented Jun 4, 2020

I need to seperate the moving from the modification to make it easier to review..

@ia3andy ia3andy force-pushed the introduce-quarkus-project branch from 56e060f to 256650b Compare June 4, 2020 13:12
@ia3andy ia3andy marked this pull request as ready for review June 4, 2020 13:17
@ia3andy ia3andy force-pushed the introduce-quarkus-project branch from 256650b to 9d4b455 Compare June 4, 2020 13:38
@gastaldi
Copy link
Contributor

gastaldi commented Jun 4, 2020

This change impacts #9593, I'll wait until this is merged to proceed

@stalep
Copy link
Member

stalep commented Jun 4, 2020

This change impacts #9593, I'll wait until this is merged to proceed

Hm, I just noticed #9593, I guess that will affect the CLI as well? I'll just try to rebase my PR on this though and we can look at it later. ATM the CLI commands do not use any registry.

@gastaldi
Copy link
Contributor

gastaldi commented Jun 4, 2020 •
edited
Loading

@stalep I think it will probably affect the classes formerly known as ListExtensionsCommandHandler and AddExtensionsCommandHandler :)

@ia3andy
Copy link
Contributor Author

ia3andy commented Jun 4, 2020

I am in PTO tomorrow, I will not be able to follow up on the discussions before monday.. really sorry :-/

@stalep @gastaldi can your PRs wait until then?

@gastaldi
Copy link
Contributor

gastaldi commented Jun 4, 2020

@ia3andy sure, I'll be refining the registry descriptor API meanwhile 😉

@stalep
Copy link
Member

stalep commented Jun 4, 2020

No problem, I just promised @maxandersen I would do a PR today :)

@maxandersen
Copy link
Member

@stalep do open the pr - better to have and then we can help rebase and get early feedback started.

@ia3andy
Copy link
Contributor Author

ia3andy commented Jun 8, 2020

I made modifications as discussed, I need to fix all the tests, I am going to push later in the afternoon..

@ia3andy ia3andy force-pushed the introduce-quarkus-project branch from 81aca5c to e185272 Compare June 8, 2020 14:44
gastaldi
gastaldi requested changes Jun 9, 2020
View reviewed changes
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.

Looks good, but the PR needs to be squashed before merged

@ia3andy
Copy link
Contributor Author

ia3andy commented Jun 9, 2020

@gastaldi I am waiting to see if CI passes then I'll merge your changes (probably tomorrow).. cheers

@ia3andy ia3andy force-pushed the introduce-quarkus-project branch 3 times, most recently from 244b9e9 to 036f5cd Compare June 9, 2020 17:07
@ia3andy
Copy link
Contributor Author

ia3andy commented Jun 9, 2020

@gastaldi changes applied (even the Collection 😅) lol and commits squashed

@ia3andy ia3andy force-pushed the introduce-quarkus-project branch from 036f5cd to f978718 Compare June 9, 2020 21:09
@ia3andy
Copy link
Contributor Author

ia3andy commented Jun 10, 2020

I am trying to figure out why CI is failing (and giving me headaches)

@ia3andy
Refactor and Introduce ExtensionsManager in CodeGen
97f9981 
This is part of quarkusio#8178

- Improved packages names and content
- Improved general readability and consistency
- Added ExtensionManager to define a high level way of managing (read/write) extensions in any QuarkusProject
- Removed most unsafe Gradle operations which were outside of the Gradle plugin (we need to figure out a way to improve the "generic" gradle support to make it compatible again..)
- Removed existing project support with the create command (throws an error) -> quarkusio#9875
- Removed compatibility with project without the Quarkus platform bom defined
- Remove SetupIT which mostly duplicate CreateProjectMojoIT
@ia3andy ia3andy force-pushed the introduce-quarkus-project branch from f978718 to 97f9981 Compare June 10, 2020 08:43
maxandersen
maxandersen reviewed Jun 10, 2020
View reviewed changes
Copy link
Member

@maxandersen maxandersen left a comment

Choose a reason for hiding this comment

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

I haven't reviewed this in full depth thus I don't feel right to explictily approve it but @gastaldi and @aloubyansky seem on top of it; but overall looks as good cleanup and I didn't spot anything alarming.

Just check if mvn io.quarkus:quarkus-maven-plugin:1.5.0.Final:create --platformGroupId=xxx --platformArtifactid=xxx --platformVersion=xxx where xxx is pointing to this version of built quarkus platform.

If need deeper review let me know :)

@ia3andy
Copy link
Contributor Author

ia3andy commented Jun 10, 2020

@gsmet can we merge without JDK14 tests passing?

@ia3andy
Copy link
Contributor Author

ia3andy commented Jun 10, 2020

--platformGroupId=xxx --platformArtifactid=xx

@maxandersen I just tested it and it works fine:

mvn io.quarkus:quarkus-maven-plugin:1.5.0.Final:create -DprojectArtifactId="test-compat" "-DplatformArtifactId=quarkus-bom" "-DplatformVersion=999-SNAPSHOT" -Dextensions="rest-client, resteasy-jsonb, logging-sentry"

@aloubyansky aloubyansky merged commit c7b624d into quarkusio:master Jun 10, 2020
@ia3andy ia3andy mentioned this pull request Jun 10, 2020
Merged
@gsmet gsmet added this to the 1.6.0 - master milestone Jun 11, 2020
@ia3andy ia3andy mentioned this pull request Jun 15, 2020
Closed
ia3andy added a commit to quarkusio/code.quarkus.io that referenced this pull request Jun 15, 2020
@ia3andy
Compat with the recent Quarkus codegen changes
1dcfc49 
This PR is to make code.quarkus ready for the codegen changes:
- quarkusio/quarkus#9787
- quarkusio/quarkus#9953
@ia3andy ia3andy mentioned this pull request Jul 1, 2020
Closed
7 tasks
gsmet pushed a commit to gsmet/code.quarkus.io that referenced this pull request Jul 8, 2020
@ia3andy @gsmet
Compat with the recent Quarkus codegen changes
c52d5ab 
This PR is to make code.quarkus ready for the codegen changes:
- quarkusio/quarkus#9787
- quarkusio/quarkus#9953
gsmet pushed a commit to quarkusio/code.quarkus.io that referenced this pull request Jul 8, 2020
@ia3andy @gsmet
Compat with the recent Quarkus codegen changes
33fc472 
This PR is to make code.quarkus ready for the codegen changes:
- quarkusio/quarkus#9787
- quarkusio/quarkus#9953
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stalep stalep stalep left review comments

@maxandersen maxandersen maxandersen left review comments

@gastaldi gastaldi gastaldi approved these changes

@aloubyansky aloubyansky aloubyansky approved these changes

Assignees
No one assigned
Labels
area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/gradle Gradle area/maven area/platform Issues related to definition and interaction with Quarkus Platform
Projects
None yet
Milestone
1.6.0.CR1
Development

Successfully merging this pull request may close these issues.
6 participants
@ia3andy @gastaldi @stalep @maxandersen @aloubyansky @gsmet