kgyrtkirk commented on a change in pull request #1161:
URL: https://github.com/apache/hive/pull/1161#discussion_r452110531



##########
File path: common/src/java/org/apache/hadoop/hive/common/jsonexplain/Vertex.java
##########
@@ -308,6 +309,16 @@ public void setType(String type) {
     this.edgeType = this.parser.mapEdgeType(type);
   }
 
+  @Override
+  public boolean equals(Object o) {
+    return  super.equals(o);

Review comment:
       interesting choices were made when this class was created...
   the most important fields seems to be `String name, JSONObject vertexObject, 
Stage stage`
   passing the `parser` in the constructor is an interesting idea...
   
   although this `equals` resorts to identity comparision - I don't see that so 
out of scope for this class...

##########
File path: 
common/src/java/org/apache/hadoop/hive/common/jsonexplain/DagJsonParserUtils.java
##########
@@ -26,9 +28,13 @@
 
 public class DagJsonParserUtils {
 
-  public static List<String> OperatorNoStats = Arrays.asList(new String[] { 
"File Output Operator",
+  private static final List<String> operatorNoStats = Arrays.asList(new 
String[] { "File Output Operator",

Review comment:
       the issue here was:
   * missing final
   * naming convention
   
   I don't think we need that static method...but no big deal

##########
File path: common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java
##########
@@ -133,6 +134,7 @@ public static void setPerfLogger(PerfLogger 
resetPerfLogger) {
    * @param callerName the logging object to be used.
    * @param method method or ID that identifies this perf log element.
    */
+  @SuppressFBWarnings(value = "NM_METHOD_NAMING_CONVENTION", justification = 
"Intended")
   public void PerfLogBegin(String callerName, String method) {

Review comment:
       it's not just "spotbugs" who don't like these method names :D
   it certainly out of scope of this patch - but I think they should be 
improved - could you open a jira for it? :)




----------------------------------------------------------------
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]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to