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

Make VCS URI annotation configurable #39554

Merged
merged 1 commit into from
Mar 21, 2024
Merged

Make VCS URI annotation configurable #39554

merged 1 commit into from
Mar 21, 2024

Conversation

xstefank
Copy link
Member

@xstefank xstefank commented Mar 19, 2024 •
edited by geoand
Loading

jcechace
jcechace reviewed Mar 19, 2024
View reviewed changes
...anilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/KubernetesCommonHelper.java

private static String parseVCSUri(VCSUriConfig config, ScmInfo scm) {
if (config.enabled) {
return config.override.orElseGet(() -> scm != null ? Git.sanitizeRemoteUrl(scm.getRemote().get("origin")) : null);
Copy link

Choose a reason for hiding this comment

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

Mirroring my comment from the original issue.

Is it reasonable to get ScmInfo directly from SCM configuration compared to SCM configuration of project's build file (e.g. pom.xml)? I really wouldn't expect application framework to go digging into my git configuration.

Copy link
Contributor

Choose a reason for hiding this comment

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

SCM configuration may or may not be configured, in contrast to .git.
Do you see any reason why the extension should not go digging to the git configuration?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I really don't understand the argument of not using .git...

@jcechace
Copy link

Left a food for thought comment. Regardless thank you for providing an override option!

@quarkus-bot
Copy link

quarkus-bot bot commented Mar 19, 2024 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 870853f.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@geoand geoand requested a review from iocanel March 20, 2024 12:11
iocanel
iocanel approved these changes Mar 21, 2024
View reviewed changes
Copy link
Contributor

@iocanel iocanel left a comment

Choose a reason for hiding this comment

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

LGTM

@geoand geoand merged commit 886b78e into quarkusio:main Mar 21, 2024
21 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.10 - main milestone Mar 21, 2024
@quarkus-bot
Copy link

quarkus-bot bot commented Mar 21, 2024

Milestone is already set for some of the items:

We haven't automatically updated the milestones for these items.

This message is automatically generated by a bot.

@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcechace jcechace jcechace left review comments

@geoand geoand geoand approved these changes

@iocanel iocanel iocanel approved these changes

Assignees
No one assigned
Labels
area/kubernetes kind/enhancement New feature or request
Projects
None yet
Milestone
3.10.0.CR1
Development

Successfully merging this pull request may close these issues.

Make annotation app.quarkus.io/vcs-uri optional in Kubernetes extension
4 participants
@xstefank @jcechace @iocanel @geoand