[GitHub] [hbase] saintstack commented on a change in pull request #1640: HBASE-24309 Avoid introducing log4j and slf4j-log4j dependencies for modules other than hbase-assembly

[GitBox]

saintstack commented on a change in pull request #1640:
URL: https://github.com/apache/hbase/pull/1640#discussion_r422369449



##
File path: hbase-assembly/pom.xml
##
@@ -318,5 +318,18 @@
   jaxws-ri
   pom
 
+

Review comment:
   I'm asking about running an hbase instance from built checkout, not from 
an assembly. Currently it works. It looks like it will no longer work if this 
patch goes in?





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:
us...@infra.apache.org

[GitHub] [hbase] saintstack commented on a change in pull request #1640: HBASE-24309 Avoid introducing log4j and slf4j-log4j dependencies for modules other than hbase-assembly

[GitBox]

saintstack commented on a change in pull request #1640:
URL: https://github.com/apache/hbase/pull/1640#discussion_r421937064



##
File path: hbase-assembly/pom.xml
##
@@ -318,5 +318,18 @@
   jaxws-ri
   pom
 
+

Review comment:
   What happens when we run in-situ... i.e. build and then do 
./bin/start-hbase.sh? Does it produce logs?

##
File path: hbase-assembly/pom.xml
##
@@ -318,5 +318,18 @@
   jaxws-ri
   pom
 
+

Review comment:
   This is a good note. Should it go elsewhere, in the top-level pom for 
instance? (Maybe it is there already...  Let me keep going)

##
File path: hbase-archetypes/hbase-client-project/pom.xml
##
@@ -43,31 +43,35 @@
 
   org.apache.hbase
   hbase-testing-util
-  ${project.version}
   test
 
 
   org.apache.hbase
   hbase-common
-  ${project.version}

Review comment:
   Good

##
File path: hbase-shaded/pom.xml
##
@@ -51,15 +51,14 @@
   
  org.apache.hbase
  hbase-resource-bundle
- ${project.version}
  true
   
-  
+  

Review comment:
   This is targeted at hbase2 which is to run against hadoop2? This comment 
that it can't be non-optional is fixed now? Not needed?

##
File path: pom.xml
##
@@ -2861,6 +2917,14 @@
org.codehause.jackson
jackson-mapper-asl
  
+ 
+   org.slf4j
+   slf4j-log4j12
+ 
+ 
+   log4j
+   log4j
+ 

Review comment:
   Painful. 

##
File path: hbase-server/pom.xml
##
@@ -481,15 +473,6 @@
   httpcore
   test
 
-
-
-  commons-logging
-  commons-logging
-  compile
-

Review comment:
   Fixed?





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:
us...@infra.apache.org