[jira] [Commented] (HBASE-24640) Purge use of VisibleForTesting
[
https://issues.apache.org/jira/browse/HBASE-24640?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17453769#comment-17453769
]
Yu Li commented on HBASE-24640:
---
Just a note here, that please use
{{com.google.errorprone.annotations.RestrictedApi}} instead when you need to
expose some methods for testing purpose in {{IA.Private}} classes. More details
please refer to HBASE-25333.
> Purge use of VisibleForTesting
> --
>
> Key: HBASE-24640
> URL: https://issues.apache.org/jira/browse/HBASE-24640
> Project: HBase
> Issue Type: Task
> Components: community
>Affects Versions: 3.0.0-alpha-1, 2.4.0
>Reporter: Nick Dimiduk
>Assignee: Andrew Kyle Purtell
>Priority: Major
> Fix For: 3.0.0-alpha-1, 1.7.0, 2.4.0
>
>
> From the dev-list thread ["[DISCUSS] VisibleForTesting annotation as it
> pertains to our API compatibility
> guidelines"|https://lists.apache.org/thread.html/rc7c7c66f134fe135d0a4454a883215e26ff3d20e5a31ecd6a2d1db77%40%3Cdev.hbase.apache.org%3E],
> when used in classes annotated with interface audience other than
> IA.Private, the VisibleForTesting annotation is confusing and considered
> harmful. The consensus is that we do not want to use this annotation as part
> of the definition of our public APIs, and we need to remove the point of
> confusion.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
[jira] [Commented] (HBASE-24640) Purge use of VisibleForTesting
[
https://issues.apache.org/jira/browse/HBASE-24640?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17239232#comment-17239232
]
Hudson commented on HBASE-24640:
Results for branch branch-2
[build #112 on
builds.a.o|https://ci-hadoop.apache.org/job/HBase/job/HBase%20Nightly/job/branch-2/112/]:
(/) *{color:green}+1 overall{color}*
details (if available):
(/) {color:green}+1 general checks{color}
-- For more information [see general
report|https://ci-hadoop.apache.org/job/HBase/job/HBase%20Nightly/job/branch-2/112/General_20Nightly_20Build_20Report/]
(/) {color:green}+1 jdk8 hadoop2 checks{color}
-- For more information [see jdk8 (hadoop2)
report|https://ci-hadoop.apache.org/job/HBase/job/HBase%20Nightly/job/branch-2/112/JDK8_20Nightly_20Build_20Report_20_28Hadoop2_29/]
(/) {color:green}+1 jdk8 hadoop3 checks{color}
-- For more information [see jdk8 (hadoop3)
report|https://ci-hadoop.apache.org/job/HBase/job/HBase%20Nightly/job/branch-2/112/JDK8_20Nightly_20Build_20Report_20_28Hadoop3_29/]
(/) {color:green}+1 jdk11 hadoop3 checks{color}
-- For more information [see jdk11
report|https://ci-hadoop.apache.org/job/HBase/job/HBase%20Nightly/job/branch-2/112/JDK11_20Nightly_20Build_20Report_20_28Hadoop3_29/]
(/) {color:green}+1 source release artifact{color}
-- See build output for details.
(/) {color:green}+1 client integration test{color}
> Purge use of VisibleForTesting
> --
>
> Key: HBASE-24640
> URL: https://issues.apache.org/jira/browse/HBASE-24640
> Project: HBase
> Issue Type: Task
> Components: community
>Affects Versions: 3.0.0-alpha-1, 2.4.0
>Reporter: Nick Dimiduk
>Assignee: Andrew Kyle Purtell
>Priority: Major
> Fix For: 3.0.0-alpha-1, 1.7.0, 2.4.0
>
>
> From the dev-list thread ["[DISCUSS] VisibleForTesting annotation as it
> pertains to our API compatibility
> guidelines"|https://lists.apache.org/thread.html/rc7c7c66f134fe135d0a4454a883215e26ff3d20e5a31ecd6a2d1db77%40%3Cdev.hbase.apache.org%3E],
> when used in classes annotated with interface audience other than
> IA.Private, the VisibleForTesting annotation is confusing and considered
> harmful. The consensus is that we do not want to use this annotation as part
> of the definition of our public APIs, and we need to remove the point of
> confusion.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (HBASE-24640) Purge use of VisibleForTesting
[
https://issues.apache.org/jira/browse/HBASE-24640?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17239216#comment-17239216
]
Hudson commented on HBASE-24640:
Results for branch master
[build #138 on
builds.a.o|https://ci-hadoop.apache.org/job/HBase/job/HBase%20Nightly/job/master/138/]:
(x) *{color:red}-1 overall{color}*
details (if available):
(/) {color:green}+1 general checks{color}
-- For more information [see general
report|https://ci-hadoop.apache.org/job/HBase/job/HBase%20Nightly/job/master/138/General_20Nightly_20Build_20Report/]
(x) {color:red}-1 jdk8 hadoop3 checks{color}
-- For more information [see jdk8 (hadoop3)
report|https://ci-hadoop.apache.org/job/HBase/job/HBase%20Nightly/job/master/138/JDK8_20Nightly_20Build_20Report_20_28Hadoop3_29/]
(x) {color:red}-1 jdk11 hadoop3 checks{color}
-- For more information [see jdk11
report|https://ci-hadoop.apache.org/job/HBase/job/HBase%20Nightly/job/master/138/JDK11_20Nightly_20Build_20Report_20_28Hadoop3_29/]
(/) {color:green}+1 source release artifact{color}
-- See build output for details.
(/) {color:green}+1 client integration test{color}
> Purge use of VisibleForTesting
> --
>
> Key: HBASE-24640
> URL: https://issues.apache.org/jira/browse/HBASE-24640
> Project: HBase
> Issue Type: Task
> Components: community
>Affects Versions: 3.0.0-alpha-1, 2.4.0
>Reporter: Nick Dimiduk
>Assignee: Andrew Kyle Purtell
>Priority: Major
> Fix For: 3.0.0-alpha-1, 1.7.0, 2.4.0
>
>
> From the dev-list thread ["[DISCUSS] VisibleForTesting annotation as it
> pertains to our API compatibility
> guidelines"|https://lists.apache.org/thread.html/rc7c7c66f134fe135d0a4454a883215e26ff3d20e5a31ecd6a2d1db77%40%3Cdev.hbase.apache.org%3E],
> when used in classes annotated with interface audience other than
> IA.Private, the VisibleForTesting annotation is confusing and considered
> harmful. The consensus is that we do not want to use this annotation as part
> of the definition of our public APIs, and we need to remove the point of
> confusion.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (HBASE-24640) Purge use of VisibleForTesting
[
https://issues.apache.org/jira/browse/HBASE-24640?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17239072#comment-17239072
]
Hudson commented on HBASE-24640:
Results for branch branch-1
[build #59 on
builds.a.o|https://ci-hadoop.apache.org/job/HBase/job/HBase%20Nightly/job/branch-1/59/]:
(x) *{color:red}-1 overall{color}*
details (if available):
(/) {color:green}+1 general checks{color}
-- For more information [see general
report|https://ci-hadoop.apache.org/job/HBase/job/HBase%20Nightly/job/branch-1/59//General_Nightly_Build_Report/]
(x) {color:red}-1 jdk7 checks{color}
-- For more information [see jdk7
report|https://ci-hadoop.apache.org/job/HBase/job/HBase%20Nightly/job/branch-1/59//JDK7_Nightly_Build_Report/]
(x) {color:red}-1 jdk8 hadoop2 checks{color}
-- For more information [see jdk8 (hadoop2)
report|https://ci-hadoop.apache.org/job/HBase/job/HBase%20Nightly/job/branch-1/59//JDK8_Nightly_Build_Report_(Hadoop2)/]
(x) {color:red}-1 source release artifact{color}
-- See build output for details.
> Purge use of VisibleForTesting
> --
>
> Key: HBASE-24640
> URL: https://issues.apache.org/jira/browse/HBASE-24640
> Project: HBase
> Issue Type: Task
> Components: community
>Affects Versions: 3.0.0-alpha-1, 2.4.0
>Reporter: Nick Dimiduk
>Assignee: Andrew Kyle Purtell
>Priority: Major
> Fix For: 3.0.0-alpha-1, 1.7.0, 2.4.0
>
>
> From the dev-list thread ["[DISCUSS] VisibleForTesting annotation as it
> pertains to our API compatibility
> guidelines"|https://lists.apache.org/thread.html/rc7c7c66f134fe135d0a4454a883215e26ff3d20e5a31ecd6a2d1db77%40%3Cdev.hbase.apache.org%3E],
> when used in classes annotated with interface audience other than
> IA.Private, the VisibleForTesting annotation is confusing and considered
> harmful. The consensus is that we do not want to use this annotation as part
> of the definition of our public APIs, and we need to remove the point of
> confusion.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (HBASE-24640) Purge use of VisibleForTesting
[ https://issues.apache.org/jira/browse/HBASE-24640?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17239026#comment-17239026 ] Duo Zhang commented on HBASE-24640: --- My point is that this is a common thing in java world so if we do not have very strong reason that this is evil(for example, it leaks guava dependency and also makes users confusing if it is used in public API), then we'd better not disable it just because of 'I do not think it is useful'. For me, I think an annotation is better than javadoc as it can enforce the same pattern and can easily be searched across the whole code base. For javadoc, since not all the developers have the same English ability, we may use different words and also easy to have typos so even if we are saying the same thing, we could see completely different javadoc sentences... Anyway, let me see if we could enforce a rule on disabling the usage of VisibleForTesting in IA.Public or IA.LimitedPrivate classes, either through maven enforer rule or yetus pre commit check. If this could be done then we could talk about allowing VisibleForTesting again, otherwise let's just disable it across the whole code base by banning the import of VisibleForTesting. Thanks. > Purge use of VisibleForTesting > -- > > Key: HBASE-24640 > URL: https://issues.apache.org/jira/browse/HBASE-24640 > Project: HBase > Issue Type: Task > Components: community >Affects Versions: 3.0.0-alpha-1, 2.4.0 >Reporter: Nick Dimiduk >Assignee: Andrew Kyle Purtell >Priority: Major > Fix For: 3.0.0-alpha-1, 1.7.0, 2.4.0 > > > From the dev-list thread ["[DISCUSS] VisibleForTesting annotation as it > pertains to our API compatibility > guidelines"|https://lists.apache.org/thread.html/rc7c7c66f134fe135d0a4454a883215e26ff3d20e5a31ecd6a2d1db77%40%3Cdev.hbase.apache.org%3E], > when used in classes annotated with interface audience other than > IA.Private, the VisibleForTesting annotation is confusing and considered > harmful. The consensus is that we do not want to use this annotation as part > of the definition of our public APIs, and we need to remove the point of > confusion. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (HBASE-24640) Purge use of VisibleForTesting
[ https://issues.apache.org/jira/browse/HBASE-24640?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17239013#comment-17239013 ] Andrew Kyle Purtell commented on HBASE-24640: - VisibleForTesting binds us to Guava for no actual benefit. What is it used for? Does an IDE use it? Or some static analysis tool? Not as far as I can see. It would be a good follow up to make a maven enforcer rule that prevents adding it back. > Purge use of VisibleForTesting > -- > > Key: HBASE-24640 > URL: https://issues.apache.org/jira/browse/HBASE-24640 > Project: HBase > Issue Type: Task > Components: community >Affects Versions: 3.0.0-alpha-1, 2.4.0 >Reporter: Nick Dimiduk >Assignee: Andrew Kyle Purtell >Priority: Major > Fix For: 3.0.0-alpha-1, 1.7.0, 2.4.0 > > > From the dev-list thread ["[DISCUSS] VisibleForTesting annotation as it > pertains to our API compatibility > guidelines"|https://lists.apache.org/thread.html/rc7c7c66f134fe135d0a4454a883215e26ff3d20e5a31ecd6a2d1db77%40%3Cdev.hbase.apache.org%3E], > when used in classes annotated with interface audience other than > IA.Private, the VisibleForTesting annotation is confusing and considered > harmful. The consensus is that we do not want to use this annotation as part > of the definition of our public APIs, and we need to remove the point of > confusion. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (HBASE-24640) Purge use of VisibleForTesting
[ https://issues.apache.org/jira/browse/HBASE-24640?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17239010#comment-17239010 ] Duo Zhang commented on HBASE-24640: --- I would say that on a IA.Private class it is fine to add VisibleForTesting annotation? It is not about API, it is just a hint to developers. Of course a javadoc would almost archive the same goal but I do not see the necessarity to disable the usage of a common usage in the java world where it is not so critical. Anyway, I'm fine with the current patch as it is difficult to filter out IA.Public and IA.LimitedPrivate class and only remove the VisibleForTesting on these classes. I will try to see if we could have some ways to disable the usage of this annotation on IA.Public classes as well as IA.LimitedPrivate classes. If this could be done, then I think it is OK for us to use it for IA.Private class then. If not, maybe we could just add a strict rule to disable all the usage of VisibleForTesting, and tell developers the reason is because we want to prevent this usage from our public API but there is no simple way to this check automatically so we decide to disable the usage through the whole code base. Thanks. > Purge use of VisibleForTesting > -- > > Key: HBASE-24640 > URL: https://issues.apache.org/jira/browse/HBASE-24640 > Project: HBase > Issue Type: Task > Components: community >Affects Versions: 3.0.0-alpha-1, 2.4.0 >Reporter: Nick Dimiduk >Assignee: Andrew Kyle Purtell >Priority: Major > Fix For: 3.0.0-alpha-1, 1.7.0, 2.4.0 > > > From the dev-list thread ["[DISCUSS] VisibleForTesting annotation as it > pertains to our API compatibility > guidelines"|https://lists.apache.org/thread.html/rc7c7c66f134fe135d0a4454a883215e26ff3d20e5a31ecd6a2d1db77%40%3Cdev.hbase.apache.org%3E], > when used in classes annotated with interface audience other than > IA.Private, the VisibleForTesting annotation is confusing and considered > harmful. The consensus is that we do not want to use this annotation as part > of the definition of our public APIs, and we need to remove the point of > confusion. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (HBASE-24640) Purge use of VisibleForTesting
[ https://issues.apache.org/jira/browse/HBASE-24640?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17237672#comment-17237672 ] Andrew Kyle Purtell commented on HBASE-24640: - [~reidchan] This looks like something I can do for 1.7.0 as well. > Purge use of VisibleForTesting > -- > > Key: HBASE-24640 > URL: https://issues.apache.org/jira/browse/HBASE-24640 > Project: HBase > Issue Type: Task > Components: community >Affects Versions: 3.0.0-alpha-1, 2.4.0 >Reporter: Nick Dimiduk >Assignee: Andrew Kyle Purtell >Priority: Major > Fix For: 3.0.0-alpha-1, 1.7.0, 2.4.0 > > > From the dev-list thread ["[DISCUSS] VisibleForTesting annotation as it > pertains to our API compatibility > guidelines"|https://lists.apache.org/thread.html/rc7c7c66f134fe135d0a4454a883215e26ff3d20e5a31ecd6a2d1db77%40%3Cdev.hbase.apache.org%3E], > when used in classes annotated with interface audience other than > IA.Private, the VisibleForTesting annotation is confusing and considered > harmful. The consensus is that we do not want to use this annotation as part > of the definition of our public APIs, and we need to remove the point of > confusion. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (HBASE-24640) Purge use of VisibleForTesting
[ https://issues.apache.org/jira/browse/HBASE-24640?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17237667#comment-17237667 ] Andrew Kyle Purtell commented on HBASE-24640: - Here is what I plan to do: * Find all VisibleForTesting annotations. * If the enclosing class or interface is IA Public or LImitedPrivate, replace the VisibleForTesting annotation with IA.Private. * If the enclosing class or interface is IA Private, remove the VisibleForTesting annotation. > Purge use of VisibleForTesting > -- > > Key: HBASE-24640 > URL: https://issues.apache.org/jira/browse/HBASE-24640 > Project: HBase > Issue Type: Task > Components: community >Affects Versions: 3.0.0-alpha-1, 2.4.0 >Reporter: Nick Dimiduk >Assignee: Andrew Kyle Purtell >Priority: Major > > From the dev-list thread ["[DISCUSS] VisibleForTesting annotation as it > pertains to our API compatibility > guidelines"|https://lists.apache.org/thread.html/rc7c7c66f134fe135d0a4454a883215e26ff3d20e5a31ecd6a2d1db77%40%3Cdev.hbase.apache.org%3E], > when used in classes annotated with interface audience other than > IA.Private, the VisibleForTesting annotation is confusing and considered > harmful. The consensus is that we do not want to use this annotation as part > of the definition of our public APIs, and we need to remove the point of > confusion. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (HBASE-24640) Purge use of VisibleForTesting
[ https://issues.apache.org/jira/browse/HBASE-24640?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17237663#comment-17237663 ] Andrew Kyle Purtell commented on HBASE-24640: - Copying from the thread discussion, the full proposal is 2. purge the VisibleForTesting annotation from our codebase for 2.4+, involving: 2a. replace VisibleForTesting with IA.Private anywhere method visibility cannot be limited 2b. perhaps add a new Yetus check that would ban new use of VisibleForTesting > Purge use of VisibleForTesting > -- > > Key: HBASE-24640 > URL: https://issues.apache.org/jira/browse/HBASE-24640 > Project: HBase > Issue Type: Task > Components: community >Affects Versions: 3.0.0-alpha-1, 2.4.0 >Reporter: Nick Dimiduk >Assignee: Andrew Kyle Purtell >Priority: Major > > From the dev-list thread ["[DISCUSS] VisibleForTesting annotation as it > pertains to our API compatibility > guidelines"|https://lists.apache.org/thread.html/rc7c7c66f134fe135d0a4454a883215e26ff3d20e5a31ecd6a2d1db77%40%3Cdev.hbase.apache.org%3E], > when used in classes annotated with interface audience other than > IA.Private, the VisibleForTesting annotation is confusing and considered > harmful. The consensus is that we do not want to use this annotation as part > of the definition of our public APIs, and we need to remove the point of > confusion. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (HBASE-24640) Purge use of VisibleForTesting
[ https://issues.apache.org/jira/browse/HBASE-24640?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17237661#comment-17237661 ] Andrew Kyle Purtell commented on HBASE-24640: - I encountered issues with VisibleForTesting on HBASE-25308 . +1 for a total removal > Purge use of VisibleForTesting > -- > > Key: HBASE-24640 > URL: https://issues.apache.org/jira/browse/HBASE-24640 > Project: HBase > Issue Type: Task > Components: community >Affects Versions: 3.0.0-alpha-1, 2.4.0 >Reporter: Nick Dimiduk >Priority: Major > > From the dev-list thread ["[DISCUSS] VisibleForTesting annotation as it > pertains to our API compatibility > guidelines"|https://lists.apache.org/thread.html/rc7c7c66f134fe135d0a4454a883215e26ff3d20e5a31ecd6a2d1db77%40%3Cdev.hbase.apache.org%3E], > when used in classes annotated with interface audience other than > IA.Private, the VisibleForTesting annotation is confusing and considered > harmful. The consensus is that we do not want to use this annotation as part > of the definition of our public APIs, and we need to remove the point of > confusion. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (HBASE-24640) Purge use of VisibleForTesting
[ https://issues.apache.org/jira/browse/HBASE-24640?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17174407#comment-17174407 ] Wellington Chevreuil commented on HBASE-24640: -- +1 for the proposal on the comment summarising [this thread discussion.|https://lists.apache.org/thread.html/r9a2df6a3b58e00c0c482d8660434d8ce6075863c18700978e6ea8b21%40%3Cdev.hbase.apache.org%3E] > Purge use of VisibleForTesting > -- > > Key: HBASE-24640 > URL: https://issues.apache.org/jira/browse/HBASE-24640 > Project: HBase > Issue Type: Task > Components: community >Affects Versions: 3.0.0-alpha-1, 2.4.0 >Reporter: Nick Dimiduk >Priority: Major > > From the dev-list thread ["[DISCUSS] VisibleForTesting annotation as it > pertains to our API compatibility > guidelines"|https://lists.apache.org/thread.html/rc7c7c66f134fe135d0a4454a883215e26ff3d20e5a31ecd6a2d1db77%40%3Cdev.hbase.apache.org%3E], > when used in classes annotated with interface audience other than > IA.Private, the VisibleForTesting annotation is confusing and considered > harmful. The consensus is that we do not want to use this annotation as part > of the definition of our public APIs, and we need to remove the point of > confusion. -- This message was sent by Atlassian Jira (v8.3.4#803005)
