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

Remove unnecessary test dependencies #15741

Merged
merged 1 commit into from
Mar 16, 2021

Conversation

glefloch
Copy link
Member

@glefloch glefloch commented Mar 15, 2021

This branch leverage all dependencies used in gradle test IT. Addresses task number 2 from #15686.

@famod, let's see how build goes :). I manually updated the pom.xml.
In the pom.xml, there are some -deployment dependencies that are not used in any gradle project (like quarkus-grpc-deployment, quarkus-kotlin-deployment) should we keep them?

@quarkus-bot quarkus-bot bot added the area/gradle Gradle label Mar 15, 2021
@famod
Copy link
Member

famod commented Mar 15, 2021

I manually updated the pom.xml.

Don't trust my script? 😄

In the pom.xml, there are some -deployment dependencies that are not used in any gradle project (like quarkus-grpc-deployment, quarkus-kotlin-deployment) should we keep them?

If the runtime counterparts are used: yes absolutely! For the same reason we enrich all other ITs with those minimal deployment dependencies: To ensure Maven builds the respective deployment module(s) before the IT module.

@famod
Copy link
Member

famod commented Mar 15, 2021

GH actions are broken ATM: https://www.githubstatus.com/incidents/s654n76c1bwr

@@ -18,13 +18,13 @@ allprojects {

subprojects{
repositories {
mavenCentral()
Copy link
Member

Choose a reason for hiding this comment

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

Ah this reminds me of something I wanted to ask you anyway: In CI we use .github/mvn-settings.xml for Maven, which uses the Google EU central mirror, but I have seen Gradle tests downloading stuff from central instead.
So I guess something is missing to pass it to Gradle in CI?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, I don't know, if this is a central mirror, I think this could be ok. I will add it too, in case of mavenCentral() failure, it will be used.

@famod famod force-pushed the fix/reduce-extension-number branch from 89d19db to 2f2a5bb Compare March 15, 2021 23:05
@famod
Copy link
Member

famod commented Mar 15, 2021

I've just rebased this branch to trigger CI now that GH Actions are back to normal. I hope you don't mind. 🙂

@famod famod mentioned this pull request Mar 15, 2021
8 tasks
@glefloch glefloch force-pushed the fix/reduce-extension-number branch from 2f2a5bb to baed9f0 Compare March 16, 2021 08:11
Copy link
Member

@famod famod left a comment

Choose a reason for hiding this comment

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

LGTM!

Not sure if anyone else should have a look before merging. I'll leave this up to you @glefloch.

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.

This is very useful, thanks for taking the time to do that!

@gsmet gsmet merged commit ed432fe into quarkusio:main Mar 16, 2021
@quarkus-bot quarkus-bot bot added this to the 1.13 - main milestone Mar 16, 2021
@famod famod linked an issue Mar 16, 2021 that may be closed by this pull request
8 tasks
@glefloch glefloch deleted the fix/reduce-extension-number branch March 19, 2021 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incremental build improvements
3 participants