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

Distrust maven settings location from system properties #28141

Merged
merged 1 commit into from
Sep 30, 2022

Conversation

AdlerFleurant
Copy link
Contributor

@AdlerFleurant AdlerFleurant commented Sep 22, 2022

Check if maven settings file exists when getting from system properties.
Null check on mavenSettings.
Fix nearby typos.

Fixes #28090

@quarkus-bot quarkus-bot bot added area/cli Related to quarkus cli (not maven/gradle/etc.) area/core area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure labels Sep 22, 2022
@quarkus-bot quarkus-bot bot added the area/jbang Issues related to when using jbang.dev with Quarkus label Sep 22, 2022
@aloubyansky
Copy link
Member

Thanks a lot @AdlerFleurant

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.

I won't comment on the change itself, @aloubyansky already did, but I have a problem with you mixing this change with a bunch of cleanup that is totally unrelated.

If you're not able to do the split the PR easily yourself using your Git superpowers, I can do it myself.

build-parent/pom.xml Outdated Show resolved Hide resolved
.github/workflows/ci-actions-incremental.yml Outdated Show resolved Hide resolved
core/runtime/src/main/java/io/quarkus/runtime/Quarkus.java Outdated Show resolved Hide resolved
@AdlerFleurant AdlerFleurant marked this pull request as draft September 22, 2022 12:44
@quarkus-bot

This comment has been minimized.

@AdlerFleurant AdlerFleurant changed the title Avoid using non existing maven settings Distrust maven settings from system properties Sep 23, 2022
@AdlerFleurant AdlerFleurant changed the title Distrust maven settings from system properties Distrust maven settings location from system properties Sep 23, 2022
@AdlerFleurant AdlerFleurant marked this pull request as ready for review September 23, 2022 18:06
@AdlerFleurant
Copy link
Contributor Author

@gsmet . I think the latest changes should address your concern. I have reduced the scope of the change. I will submit another change for not using the deprecated API.

@AdlerFleurant AdlerFleurant requested a review from gsmet September 23, 2022 20:36
@AdlerFleurant
Copy link
Contributor Author

@gsmet, how can I help this move forward?

@AdlerFleurant
Copy link
Contributor Author

@gsmet, @aloubyansky, I think we can remove some of these labels as the scope of the change was reduced to just cli.

@gsmet
Copy link
Member

gsmet commented Sep 28, 2022

Yeah sorry I was busy on other things.

BTW, maybe I wasn't clear, but please push the other changes to another pull request. Cleanup is always welcome.

@gsmet
Copy link
Member

gsmet commented Sep 28, 2022

I will rebase as somehow CI didn't start.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Check if maven settings file exists when getting from system properties.
Null check on mavenSettings.
Fix nearby typos.

Fixes quarkusio#28090

Signed-off-by: Adler Fleurant <[email protected]>
@gsmet gsmet added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Sep 28, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Sep 28, 2022

Failing Jobs - Building 2b0923f

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
✔️ JVM Tests - JDK 17
JVM Tests - JDK 17 MacOS M1 Set up runner ⚠️ Check → Logs Raw logs
✔️ JVM Tests - JDK 18

@aloubyansky aloubyansky merged commit 4b144c6 into quarkusio:main Sep 30, 2022
@quarkus-bot quarkus-bot bot added this to the 2.14 - main milestone Sep 30, 2022
@quarkus-bot quarkus-bot bot added kind/bugfix and removed triage/waiting-for-ci Ready to merge when CI successfully finishes labels Sep 30, 2022
@gsmet gsmet modified the milestones: 2.14 - main, 2.13.1.Final Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli Related to quarkus cli (not maven/gradle/etc.) area/core area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure area/jbang Issues related to when using jbang.dev with Quarkus kind/bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cli test fails without settings.xml
3 participants