kezhenxu94 commented on a change in pull request #50:
URL: https://github.com/apache/skywalking-eyes/pull/50#discussion_r676221322
##########
File path: pkg/deps/maven.go
##########
@@ -0,0 +1,162 @@
+package deps
Review comment:
Missing license header, `go run cmd/license-eye/main.go header fix`
should do the trick 😉
##########
File path: pkg/deps/maven.go
##########
@@ -0,0 +1,162 @@
+package deps
+
+import (
+ "archive/zip"
+ "bufio"
+ "bytes"
+ "fmt"
+ "io"
+ "io/fs"
+ "io/ioutil"
+ "os"
+ "os/exec"
+ "path/filepath"
+ "regexp"
+
+ "github.com/apache/skywalking-eyes/license-eye/internal/logger"
+ "github.com/apache/skywalking-eyes/license-eye/pkg/license"
+)
+
+func init() {
+ var err error
+ _, err = exec.Command("mvn", "--version").Output()
+ if err != nil {
+ return
+ }
+
+ Resolvers = append(Resolvers, new(MavenPomResolver))
Review comment:
I think this is not a good condition to check whether we should add the
resolver:
- `mvn version` is not enough to determine whether maven resolver can be
use, [maven wrapper](https://github.com/takari/maven-wrapper) is also widely
used in some projects.
- We should log some warning / error logs when users' environment don't have
maven installed, instead of failing silently.
My suggestion is
- just add the resolver to
https://github.com/apache/skywalking-eyes/blob/d6e232b01042d112b103ae6c7f51c5c677e1f37e/pkg/deps/resolve.go#L29-L32
- check whether `mvnw` exists in the same directory as `pom.xml` first, if
yes, use `mvnw`, otherwise, use `mvn`
- run the needed command (`mvn copy-dependencies`), if it failed, we also
fail the command and print some logs
##########
File path: .licenserc.yaml
##########
@@ -83,3 +83,4 @@ header: # `header` section is configurations for source codes
license header.
dependency:
files:
- go.mod
+ - test/testdata/pom.xml
Review comment:
This file `.licenserc.yaml` is for real use case to check the licenses
of this repo itself, not for tests, so let’s move the test elsewhere
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
