kezhenxu94 commented on code in PR #12777:
URL: https://github.com/apache/skywalking/pull/12777#discussion_r1844884161
##########
oap-server/server-library/library-async-profiler-jfr-parser/pom.xml:
##########
@@ -30,4 +30,23 @@
<artifactId>library-async-profiler-jfr-parser</artifactId>
+ <dependencies>
+ <dependency>
+ <groupId>tools.profiler</groupId>
+ <artifactId>async-profiler-converter</artifactId>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-checkstyle-plugin</artifactId>
+ <configuration>
+ <skip>true</skip>
Review Comment:
This skips all classes in the entire module, which is not what we want
##########
pom.xml:
##########
@@ -175,6 +175,7 @@
<protoc-gen-grpc-java.plugin.version>1.42.1</protoc-gen-grpc-java.plugin.version>
<javax.annotation-api.version>1.3.2</javax.annotation-api.version>
<objenesis.version>3.1</objenesis.version>
+
<async-profiler-converter.version>3.0</async-profiler-converter.version>
Review Comment:
Move this and the `<dependency>` section to `oap-server-bom/pom.xml`
##########
LICENSE:
##########
@@ -225,4 +225,8 @@ The text of each license is the standard Apache 2.0 license.
mvnw files from https://github.com/apache/maven-wrapper Apache 2.0
svg files from skywalking-ui/src/assets/icons:
https://github.com/google/material-design-icons Apache 2.0
ZipkinQueryHandler.java reference from
zipkin2.server.internal.ZipkinQueryApiV2: https://github.com/openzipkin/zipkin
Apache 2.0
- async-profiler parser files in
server-library/library-async-profiler-jfr-parser from
https://github.com/async-profiler/async-profiler/tree/v3.0 Apache 2.0
+ Arguments.java in server-library/library-async-profiler-jfr-parser
reference from https://github.com/async-profiler/async-profiler/tree/master
Apache 2.0
+ CallStack.java in server-library/library-async-profiler-jfr-parser
reference from https://github.com/async-profiler/async-profiler/tree/master
Apache 2.0
+ Classifier.java in server-library/library-async-profiler-jfr-parser
reference from https://github.com/async-profiler/async-profiler/tree/master
Apache 2.0
Review Comment:
Also, I saw that this class `Classifier.java` is actually not copied as is,
you made some modifications to it without explicitly stated in the license, if
this class can be replaced by the package in
https://github.com/apache/skywalking/pull/12671#discussion_r1823722484 we
should be OK, but if copied and modifications are required, please add some
notes here about what is modified.
##########
LICENSE:
##########
@@ -225,4 +225,8 @@ The text of each license is the standard Apache 2.0 license.
mvnw files from https://github.com/apache/maven-wrapper Apache 2.0
svg files from skywalking-ui/src/assets/icons:
https://github.com/google/material-design-icons Apache 2.0
ZipkinQueryHandler.java reference from
zipkin2.server.internal.ZipkinQueryApiV2: https://github.com/openzipkin/zipkin
Apache 2.0
- async-profiler parser files in
server-library/library-async-profiler-jfr-parser from
https://github.com/async-profiler/async-profiler/tree/v3.0 Apache 2.0
+ Arguments.java in server-library/library-async-profiler-jfr-parser
reference from https://github.com/async-profiler/async-profiler/tree/master
Apache 2.0
+ CallStack.java in server-library/library-async-profiler-jfr-parser
reference from https://github.com/async-profiler/async-profiler/tree/master
Apache 2.0
+ Classifier.java in server-library/library-async-profiler-jfr-parser
reference from https://github.com/async-profiler/async-profiler/tree/master
Apache 2.0
+ Index.java in server-library/library-async-profiler-jfr-parser reference
from https://github.com/async-profiler/async-profiler/tree/master Apache 2.0
+ Frame.java in server-library/library-async-profiler-jfr-parser reference
from https://github.com/async-profiler/async-profiler/tree/master Apache 2.0
Review Comment:
I knew these classes are lack of java package names that makes them not easy
to be imported, and I added another async-profiler module that solves the
package name issues
https://github.com/apache/skywalking/pull/12671#discussion_r1823722484 , can
you take a look?
--
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]
