squakez commented on code in PR #5126:
URL: https://github.com/apache/camel-k/pull/5126#discussion_r1474033347
##########
pkg/util/maven/maven_project_test.go:
##########
@@ -181,6 +181,17 @@ func TestParseGAVMvnNoVersion(t *testing.T) {
assert.Equal(t, dep.ArtifactID, "camel-core")
}
+func TestParseGAVMvnClassifierNoVersion(t *testing.T) {
+ dep, err := ParseGAV("org.apache.camel:camel-k-core:jar:custom")
Review Comment:
I'd do a permutation of the possible cases to make sure we're not missing
something. We should have at least 7 different cases to test.
##########
pkg/apis/camel/v1/maven_types.go:
##########
@@ -81,14 +81,18 @@ type RepositoryPolicy struct {
ChecksumPolicy string `xml:"checksumPolicy,omitempty"
json:"checksumPolicy,omitempty"`
}
-// MavenArtifact defines a GAV (Group:Artifact:Version) Maven artifact.
+// MavenArtifact defines a GAV (Group:Artifact:Classifier:Version) Maven
artifact.
Review Comment:
Should we have the `type` as well here?
##########
pkg/trait/jolokia.go:
##########
@@ -124,7 +114,7 @@ func (t *jolokiaTrait) Apply(e *Environment) error {
jolokiaFilepath := ""
for _, ar := range e.IntegrationKit.Status.Artifacts {
- if strings.HasPrefix(ar.ID, "org.jolokia.jolokia-jvm") {
+ if strings.HasPrefix(ar.ID, "org.jolokia.jolokia-agent-jvm") {
Review Comment:
We should not change this until the new catalog is ready. Let's make it
compatible with the catalog we are using and we'll change to the new values
once we have the new catalog.
##########
pkg/apis/camel/v1/maven_types_support.go:
##########
@@ -19,12 +19,18 @@ package v1
import (
"encoding/xml"
+ "fmt"
)
func (in *MavenArtifact) GetDependencyID() string {
+ fmt.Printf(">>> maven_types_support GetDependencyID: +%+v\n", in)
Review Comment:
Debug line.
##########
pkg/util/camel/camel_dependencies.go:
##########
@@ -298,6 +298,7 @@ func addDependenciesFromCatalog(project *maven.Project,
catalog *RuntimeCatalog)
md := maven.Dependency{
GroupID: dep.GroupID,
ArtifactID: dep.ArtifactID,
+ Classifier: dep.Classifier,
Review Comment:
If we're having the `type` parameter, I think it makes sense to include it
here as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
