stoty commented on a change in pull request #887:
URL: https://github.com/apache/phoenix/pull/887#discussion_r493232427



##########
File path: phoenix-tracing-webapp/pom.xml
##########
@@ -155,6 +131,7 @@
         <plugin>
           <groupId>com.github.searls</groupId>
           <artifactId>jasmine-maven-plugin</artifactId>
+          <version>2.2</version>

Review comment:
       You are overriding the version set in the master pom pluginManagement 
section.
   I suggest that you override the plugin version there instead.

##########
File path: phoenix-tracing-webapp/src/build/trace-server-runnable.xml
##########
@@ -43,7 +43,19 @@
       <outputDirectory>/</outputDirectory>
       <includes>
         <include>org.apache.phoenix:phoenix-tracing-webapp</include>

Review comment:
       Ideally, this should be done by the shade plugin instead. (Not required 
for this patch, just a remark)

##########
File path: phoenix-tracing-webapp/README.md
##########
@@ -3,10 +3,10 @@
  *mvn clean install*

Review comment:
       Nit: We use backticks to format command lines elsewhere, might as well 
switch to that 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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to