Apache9 commented on a change in pull request #2675:
URL: https://github.com/apache/hbase/pull/2675#discussion_r537036842
##########
File path:
hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionCoprocessorHost.java
##########
@@ -79,19 +81,36 @@
@Before
public void setup() throws IOException {
+ init(null);
+ }
+
+ public void init(Boolean flag) throws IOException {
Review comment:
Let's use private here? I do not think this method needs to be called in
other classes?
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
##########
@@ -102,6 +102,11 @@
// optimization: no need to call postScannerFilterRow, if no coprocessor
implements it
private final boolean hasCustomPostScannerFilterRow;
+ @VisibleForTesting
Review comment:
OK, VisibleForTesting...
We have a dicussion thread in the mailing list, and @apurtell concluded that
we do not use annotation at all, just use a javadoc, and had already done a PR
to remove all the VisibleForTesting annotations from our code base. I'm still
trying to convince him to allow this annotation on IA.Private classes.
There is still no consensus yet, you can see the discussion in this PR
https://github.com/apache/hbase/pull/2714
So here I suggest that we remove this annotation to align with the current
rule in our code base first. But anyway, I think this is an example that
developers want to use this annotation, so maybe @apurtell could change his
opinion.
Thanks.
----------------------------------------------------------------
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]
