-
Notifications
You must be signed in to change notification settings - Fork 45
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
✨ add ability to pass maven coordinate in location #617
Conversation
docs/providers.md
Outdated
@@ -92,7 +92,7 @@ Here's an example config for `java` provider that is currently in-tree and does | |||
} | |||
``` | |||
|
|||
The `location` can be a path to the application's source code or to a binary JAR, WAR, or EAR file. | |||
The `location` can be a path to the application's source code or to a binary JAR, WAR, or EAR file. Optionally, coordinates to a maven artifact can be provided as input in the format `mvn://<group-id>.<artifact-id>:<version>:<classifier>:<path>`. The field `<path>` is optional, it specifies a local path where the artifact will be downloaded. If not specified, provider will use a temporary directory to download 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.
can we use something like @
or something that is not a part of a common URI for the path part?
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 is a minor concern
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.
sure can
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.
Looks good, just one question
if strings.HasPrefix(config.Location, MvnURIPrefix) { | ||
mvnUri := strings.Replace(config.Location, MvnURIPrefix, "", 1) | ||
// URI format is <group>:<artifact>:<version>:<classifier>:<path> | ||
// <path> is optional & points to a local path where it will be downloaded |
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.
Is the classifier mandatory in the URL?
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.
@jmle yeah, in the hub UI, we have four fields including classifier (defaulted to JAR). I think I will default to JAR too if one isn't specified
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.
@jmle done, defaulting to jar now
Signed-off-by: Pranav Gaikwad <[email protected]>
Signed-off-by: Pranav Gaikwad <[email protected]>
Fixes #554