- 
                Notifications
    You must be signed in to change notification settings 
- Fork 3.4k
HBASE-22571 Javadoc Warnings related to @return tag #302
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| 💔 -1 overall 
 
 
 This message was automatically generated. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM overall, but left a few comments. Please also address the Checkstyle issues from the HBase robot.
        
          
                hbase-client/src/main/java/org/apache/hadoop/hbase/client/SimpleRequestController.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableInputFormat.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                hbase-server/src/main/java/org/apache/hadoop/hbase/client/locking/EntityLock.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestOpenTableInCoprocessor.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/BalancerTestBase.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStore.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStoreFile.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                hbase-server/src/test/java/org/apache/hadoop/hbase/util/test/LoadTestDataGenerator.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
      | Thanks for the kind comments. I have addressed and fixed things according to comments. Kindly have a look in the 2nd commit. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some additional comments with minor improvements.
        
          
                hbase-server/src/test/java/org/apache/hadoop/hbase/TestServerSideScanMetricsFromClientSide.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStore.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStoreFile.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableInputFormat.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
      | Thanks for the feedback. Please find the corrections in the 3rd commit. | 
| 🎊 +1 overall 
 
 This message was automatically generated. | 
| 🎊 +1 overall 
 
 This message was automatically generated. | 
| @SyedMurtazaHassan Could you please resolve the merge conflicts. | 
bc1a635    to
    c61e2b9      
    Compare
  
    | @HorizonNet Solved the merge conflict, kindly review. | 
| 💔 -1 overall 
 
 
 This message was automatically generated. | 
| @SyedMurtazaHassan It seems that you added new Checkstyle issues. Would be great if you could resolve them. | 
| @HorizonNet Fixed the checkstyle issues. Kindly have a look. | 
| 💔 -1 overall 
 
 
 This message was automatically generated. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Nice cleanup.
Presuming @HorizonNet 's comments addressed.
| * @param loc | ||
| * @param heapSizeOfRow | ||
| * @return | ||
| * @return either Include {@link ReturnCode} or Skip {@link ReturnCode} | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another option if the parameter or return code are self-explanatory is to just remove them from javadoc. Perhaps not here but in the below where you add the text 'the row key' as description on a param named 'key'...
| * Returns the version components | ||
| * Examples: "1.4.3" returns [1, 4, 3], "4.5.6-SNAPSHOT" returns [4, 5, 6, "SNAPSHOT"] | ||
| * @returns the components of the version string | ||
| * @return the components of the version string | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good
| Re-running build just to make sure test failure flakey. | 
| 💔 -1 overall 
 
 
 This message was automatically generated. | 
        
          
                hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedAction.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
      | 🎊 +1 overall 
 
 This message was automatically generated. | 
| 🎊 +1 overall 
 
 This message was automatically generated. | 
Javadoc Warnings related to @return tag