I've restored the previous method signature to LoadIncrementalHFiles, so that piece is complete for the next RC.
Based on the latest comments, let me update my proposed course of action. 1. restore any VisibleForTesting method signatures for 2.3.0, treat this as public API going forward. 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 I have filed https://issues.apache.org/jira/browse/HBASE-24640 as a tracking task for this work. If the above process is agreeable, let's add these steps as a comment for future reference. Thanks, Nick On Fri, Jun 26, 2020 at 8:38 AM 张铎(Duo Zhang) <[email protected]> wrote: > For the LoadIncrementalHFiles class, the IA.Public annotation itself is a > problem. It should be IA.LimitPrivate(TOOLS). So I'm fine with either > adding the method back or not, the opinion of the release manager is > most important I think. > > Thanks. > > Viraj Jasani <[email protected]> 于2020年6月26日周五 下午9:50写道： > > > Agree on replacing VFT in next 2.4.0 or 3.0.0 release and restoring the > > required > > method for now to unblock 2.3.0 RC1. > > > > > > On 2020/06/26 01:11:31, Andrew Purtell <[email protected]> wrote: > > > Sounds fine to me. > > > > > > My earlier objection was to talk of an HBase 3 followed by an HBase 4. > We > > > don't need to do a full deprecation cycle across two major versions to > > > remove an annotation that never promised public access. (By definition, > > > tagged fields and members were VisibleForTesting (only). The 'only' was > > > implied, but I think a reasonable assumption and common knowledge.) > > > > > > On Thu, Jun 25, 2020 at 3:48 PM Sean Busbey <[email protected]> wrote: > > > > > > > Agree on restoring the member and then getting this done for 2.4.0. > > > > > > > > > > > > On Thu, Jun 25, 2020, 15:02 Nick Dimiduk <[email protected]> > wrote: > > > > > > > > > And now by module, > > > > > > > > > > $ find . -iname '*.java' -exec grep -n '@VisibleForTesting' {} \+ | > > cut > > > > -d/ > > > > > -f2 | sort | uniq -c > > > > > 6 hbase-backup > > > > > 87 hbase-client > > > > > 40 hbase-common > > > > > 1 hbase-endpoint > > > > > 7 hbase-hadoop-compat > > > > > 3 hbase-http > > > > > 18 hbase-mapreduce > > > > > 1 hbase-metrics-api > > > > > 24 hbase-procedure > > > > > 10 hbase-replication > > > > > 456 hbase-server > > > > > 2 hbase-thrift > > > > > 1 hbase-zookeeper > > > > > > > > > > I prefer we not make this change a prerequisite to 2.3. I would > > rather we > > > > > restore the one method modified by HBASE-24221 and do the work for > > > > > VisibleForTesting for 2.4.0. > > > > > > > > > > On Thu, Jun 25, 2020 at 12:57 PM Nick Dimiduk <[email protected] > > > > > > wrote: > > > > > > > > > > > On Thu, Jun 25, 2020 at 12:36 PM Andrew Purtell < > > [email protected]> > > > > > > wrote: > > > > > > > > > > > >> I think we are in agreement except for a need to have a > > deprecation > > > > > cycle. > > > > > >> Just remove VisibleForTesting and replace with whatever > > alternative > > > > you > > > > > >> like. Certainly in the next minors. No strong opinion either way > > about > > > > > >> patch releases, leave as is? > > > > > >> > > > > > > > > > > > > Thanks Andrew and Bharath, I now better understand your > positions. > > > > > > > > > > > > The annotation is fairly common in our codebase, from branch-2.3, > > > > > > > > > > > > $ find . -iname '*.java' -exec grep -n '@VisibleForTesting' {} \+ > > | wc > > > > -l > > > > > > 668 > > > > > > > > > > > > I don't have an easy way to cross-reference this with our AI > > > > annotations, > > > > > > but my concern is that any change we make here without a > > deprecation > > > > > cycle > > > > > > will be disruptive to users. > > > > > > > > > > > > On Thu, Jun 25, 2020 at 11:30 AM Nick Dimiduk < > [email protected] > > > > > > > > wrote: > > > > > >> > > > > > >> > On Wed, Jun 24, 2020 at 3:19 PM Andrew Purtell < > > [email protected] > > > > > > > > > > >> > wrote: > > > > > >> > > > > > > >> > > It is possible some users may not understand what Guava's > > > > > >> > VisibleForTesting > > > > > >> > > implies, but those users are much more likely to be Java > > > > developers > > > > > or > > > > > >> > Java > > > > > >> > > developer adjacent, and familiar with what this fad > entailed. > > Such > > > > > >> > tagging > > > > > >> > > was/is done specifically to indicate the exposed field or > > method > > > > was > > > > > >> only > > > > > >> > > made to allow test access to internals, as something less > than > > > > > public. > > > > > >> > > > > > > > >> > > For us to treat such annotated fields and methods as public > > after > > > > > all > > > > > >> is > > > > > >> > > unnecessary, possibly surprising, and not semantically sound > > > > (IMHO). > > > > > >> > > > > > > > >> > > > > > > >> > I don't want to preserve use of VisibleForTesting as an > > indicator of > > > > > >> public > > > > > >> > API. I want to ensure that we're clear to our downstream users > > > > > >> > that its presence is not a factor in determining public API. > For > > > > > >> example, I > > > > > >> > don't want to update our book to give any meaning to this > > > > annotation, > > > > > >> and I > > > > > >> > don't want to update our javadoc filters to take it into > account > > > > when > > > > > >> > generating the various versions of javadoc that we publish. I > > want > > > > to > > > > > >> purge > > > > > >> > it from the discussion by annotating the methods it decorates > > with > > > > the > > > > > >> > symbols we do use to define our public API. The steps I > propose > > > > above > > > > > >> are > > > > > >> > my suggestion of how we work toward that goal. > > > > > >> > > > > > > >> > Does anyone have a counter-proposal to the steps I've outlined > > > > above? > > > > > A > > > > > >> > resolution to this discussion is now the final blocker on > > 2.3.0rc1. > > > > > >> > > > > > > >> > Thanks, > > > > > >> > Nick > > > > > >> > > > > > > >> > On Wed, Jun 24, 2020 at 2:53 PM Sean Busbey < > [email protected]> > > > > > wrote: > > > > > >> > > > > > > > >> > > > Andrew are you specifically opposed to using a deprecation > > cycle > > > > > to > > > > > >> > > > formally label as private anything that currently has a > > > > > >> > VisibleForTesting > > > > > >> > > > annotation? > > > > > >> > > > > > > > > >> > > > On Wed, Jun 24, 2020, 16:07 Andrew Purtell < > > [email protected] > > > > > > > > > > >> > wrote: > > > > > >> > > > > > > > > >> > > > > I am -1 on treating VisibleForTesting as public API. > > > > > Semantically > > > > > >> it > > > > > >> > > > makes > > > > > >> > > > > no sense. > > > > > >> > > > > > > > > > >> > > > > On Wed, Jun 24, 2020 at 12:22 PM Nick Dimiduk < > > > > > >> [email protected]> > > > > > >> > > > wrote: > > > > > >> > > > > > > > > > >> > > > > > I'd like to get this [DISCUSS] wrapped up so we can > > proceed > > > > > with > > > > > >> > > > release > > > > > >> > > > > > candidates. > > > > > >> > > > > > > > > > > >> > > > > > I don't see a clear consensus here. The conclusion I > > read is > > > > > >> that > > > > > >> > > > > > developers generally intended the VisibleForTesting > > > > annotation > > > > > >> to > > > > > >> > > > > indicate > > > > > >> > > > > > a method is not a part of our public API, but because > we > > > > don't > > > > > >> > > > explicitly > > > > > >> > > > > > say this in our guide, we can't really stand on that > > for the > > > > > >> > > community. > > > > > >> > > > > > > > > > > >> > > > > > I propose we take the following, conservative steps > > going > > > > > >> forward: > > > > > >> > > > > > > > > > > >> > > > > > 1. restore any VisibleForTesting method signatures for > > > > 2.3.0, > > > > > >> treat > > > > > >> > > > this > > > > > >> > > > > as > > > > > >> > > > > > public API going forward. > > > > > >> > > > > > 2. annotate any existing methods carrying the > > > > > VisibleForTesting > > > > > >> > > > > annotation > > > > > >> > > > > > as Deprecated in 2.3.x+, for removal in 4.0.0 > > > > > >> > > > > > 3. purge the VisibleForTesting annotation from our > > codebase > > > > > for > > > > > >> > > 4.0.0, > > > > > >> > > > > > involving: > > > > > >> > > > > > 3a. replace VisibleForTesting with IA.Private anywhere > > > > method > > > > > >> > > > visibility > > > > > >> > > > > > cannot be limited > > > > > >> > > > > > 3b. perhaps add a new Yetus check that would ban new > > use of > > > > > >> > > > > > VisibleForTesting > > > > > >> > > > > > > > > > > >> > > > > > Did I miss anything? > > > > > >> > > > > > > > > > > >> > > > > > Thanks, > > > > > >> > > > > > Nick > > > > > >> > > > > > > > > > > >> > > > > > On Wed, Jun 24, 2020 at 12:22 AM Viraj Jasani < > > > > > >> [email protected]> > > > > > >> > > > > wrote: > > > > > >> > > > > > > > > > > >> > > > > > > +1 to "be clear in javadoc" and to the fact that > guava > > > > > >> dependency > > > > > >> > > > just > > > > > >> > > > > to > > > > > >> > > > > > > express intention which can be done through javadoc > > is not > > > > > >> > > > > > > required unless the library is capable of breaking > > > > > >> compilation of > > > > > >> > > > > > > downstream > > > > > >> > > > > > > projects if they use VFT annotated classes/methods > > saying > > > > > you > > > > > >> > can't > > > > > >> > > > use > > > > > >> > > > > > > this > > > > > >> > > > > > > (what if we have such fancy thing? :) ). > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > On 2020/06/23 20:01:40, Sean Busbey < > > [email protected]> > > > > > >> wrote: > > > > > >> > > > > > > > +1 to "do it in javadoc" unless there's some magic > > for > > > > > IDEs > > > > > >> > > brought > > > > > >> > > > > > about > > > > > >> > > > > > > > via the VFT annotation that I'm missing. > > > > > >> > > > > > > > > > > > > >> > > > > > > > On Tue, Jun 23, 2020, 13:04 Andrew Purtell < > > > > > >> > [email protected]> > > > > > >> > > > > > wrote: > > > > > >> > > > > > > > > > > > > >> > > > > > > > > I don't find the VisibleForTesting annotation > > > > provides a > > > > > >> lot > > > > > >> > of > > > > > >> > > > > > value. > > > > > >> > > > > > > It > > > > > >> > > > > > > > > became fashionable to use this annotation when a > > > > single > > > > > >> line > > > > > >> > of > > > > > >> > > > > > Javadoc > > > > > >> > > > > > > > > would serve the same purpose and not make yet > > another > > > > > >> > > dependency > > > > > >> > > > on > > > > > >> > > > > > > Guava. > > > > > >> > > > > > > > > My advice is to remove them all and replace with > > > > > Javadoc. > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > Even if in an IA.Public or LimitedPrivate we can > > > > > decorate > > > > > >> > > > > individual > > > > > >> > > > > > > > > field/methods that are public but not intended > to > > be > > > > > part > > > > > >> of > > > > > >> > > the > > > > > >> > > > > > public > > > > > >> > > > > > > > > portion of the API with a field or method level > > > > > IA.Private > > > > > >> > > > > > decoration. > > > > > >> > > > > > > It's > > > > > >> > > > > > > > > maybe not nice to do, but that would directly > and > > > > > clearly > > > > > >> > > express > > > > > >> > > > > > > intent. > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > On Tue, Jun 23, 2020 at 10:15 AM Sean Busbey < > > > > > >> > > [email protected]> > > > > > >> > > > > > > wrote: > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > I think the intent behind VisibleForTesting is > > > > clear: > > > > > >> the > > > > > >> > > > person > > > > > >> > > > > > > using > > > > > >> > > > > > > > > that > > > > > >> > > > > > > > > > annotation does not intend for it to be used > by > > > > > >> > > downstreamers. > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > However, our stated API promises are in terms > > of the > > > > > >> > > Interface > > > > > >> > > > > > > Audience > > > > > >> > > > > > > > > > annotations only. So I think a downsteamer who > > e.g. > > > > > used > > > > > >> > > > > automated > > > > > >> > > > > > > > > tooling > > > > > >> > > > > > > > > > to ensure they only used things marked > IA.Public > > > > would > > > > > >> be > > > > > >> > > > correct > > > > > >> > > > > > to > > > > > >> > > > > > > be > > > > > >> > > > > > > > > > upset with us if we incompatibly changed an > > > > IA.Public > > > > > >> > member > > > > > >> > > > that > > > > > >> > > > > > is > > > > > >> > > > > > > > > > annotated VisibleForTesting. > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > Given that VisibleForTesting is in guava and > we > > go > > > > to > > > > > >> pains > > > > > >> > > to > > > > > >> > > > > > about > > > > > >> > > > > > > > > > exposing downstream to non-relocated guava I > > think > > > > it > > > > > >> would > > > > > >> > > be > > > > > >> > > > a > > > > > >> > > > > > bad > > > > > >> > > > > > > idea > > > > > >> > > > > > > > > > to use it when defining our public API. > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > We should find places that use it, make sure > > they > > > > also > > > > > >> > carry > > > > > >> > > an > > > > > >> > > > > > > > > IA.Private > > > > > >> > > > > > > > > > if needed, and make sure our docs for > > developers are > > > > > >> clear > > > > > >> > > > about > > > > > >> > > > > > > which > > > > > >> > > > > > > > > > annotations carry meaning for downstreamers > > (i.e. > > > > only > > > > > >> > > > Interface > > > > > >> > > > > > > Audience > > > > > >> > > > > > > > > > and Interface Stability). > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > On Tue, Jun 23, 2020, 11:29 Nick Dimiduk < > > > > > >> > > [email protected]> > > > > > >> > > > > > > wrote: > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > My hope is that we can clarify our policy > and > > > > update > > > > > >> the > > > > > >> > > book > > > > > >> > > > > > > > > > accordingly. > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > On Tue, Jun 23, 2020 at 9:01 AM Wellington > > > > > Chevreuil < > > > > > >> > > > > > > > > > > [email protected]> wrote: > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > For the current problem, what is the > > class? I > > > > > >> think > > > > > >> > > > > changing > > > > > >> > > > > > a > > > > > >> > > > > > > > > method > > > > > >> > > > > > > > > > > > > signature for a protected method will > only > > > > break > > > > > >> the > > > > > >> > > > > > > compatibility > > > > > >> > > > > > > > > > when > > > > > >> > > > > > > > > > > > > users extend the class. > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > This specific case is > > > > > >> > > > > > *LoadIncrementalHFiles.tryAtomicRegionLoad, > > > > > >> > > > > > > > > > *mostly > > > > > >> > > > > > > > > > > > an end user tool, not likely to be > extended. > > > > Bring > > > > > >> back > > > > > >> > > the > > > > > >> > > > > > > original > > > > > >> > > > > > > > > > > method > > > > > >> > > > > > > > > > > > would not be much of an issue, though, I > > guess > > > > the > > > > > >> > > > discussion > > > > > >> > > > > > is > > > > > >> > > > > > > more > > > > > >> > > > > > > > > > on > > > > > >> > > > > > > > > > > > how to interpret @VisibleForTesting in > > general. > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > > > Em ter., 23 de jun. de 2020 às 15:42, > 张铎(Duo > > > > > Zhang) > > > > > >> < > > > > > >> > > > > > > > > > > [email protected] > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > escreveu: > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > Technically, I do not think the > developer > > who > > > > > >> makes a > > > > > >> > > > field > > > > > >> > > > > > or > > > > > >> > > > > > > > > method > > > > > >> > > > > > > > > > > > > public for an IA.Public class but marks > it > > > > with > > > > > >> > > > > > > @VisibleForTesting, > > > > > >> > > > > > > > > > > > > actually wants to expose this field or > > method > > > > to > > > > > >> end > > > > > >> > > > users. > > > > > >> > > > > > > > > > > > > But this could be a problem for end > > users, so > > > > I > > > > > >> think > > > > > >> > > we > > > > > >> > > > > > should > > > > > >> > > > > > > > > avoid > > > > > >> > > > > > > > > > > > doing > > > > > >> > > > > > > > > > > > > this on an IA.Public class in the > future. > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > For the current problem, what is the > > class? I > > > > > >> think > > > > > >> > > > > changing > > > > > >> > > > > > a > > > > > >> > > > > > > > > method > > > > > >> > > > > > > > > > > > > signature for a protected method will > only > > > > break > > > > > >> the > > > > > >> > > > > > > compatibility > > > > > >> > > > > > > > > > when > > > > > >> > > > > > > > > > > > > users extend the class. In fact, most of > > the > > > > > >> classes > > > > > >> > in > > > > > >> > > > our > > > > > >> > > > > > > public > > > > > >> > > > > > > > > > API > > > > > >> > > > > > > > > > > > are > > > > > >> > > > > > > > > > > > > not designed to be extended by end > users. > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > Thanks. > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > Wellington Chevreuil < > > > > > >> [email protected] > > > > > >> > > > > > > > >> > > > > > > > > 于2020年6月23日周二 > > > > > >> > > > > > > > > > > > > 下午10:33写道： > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > My opinion expressed on the 2.3.0RC0 > > thread > > > > > was > > > > > >> > that > > > > > >> > > > > > > > > > > @VisibleForTesting > > > > > >> > > > > > > > > > > > > > would flag class/method/variable as > > > > private. I > > > > > >> > > believe > > > > > >> > > > > the > > > > > >> > > > > > > > > > annotation > > > > > >> > > > > > > > > > > > > label > > > > > >> > > > > > > > > > > > > > is pretty suggestive and (I also > > believe) > > > > it's > > > > > >> > common > > > > > >> > > > > sense > > > > > >> > > > > > > that > > > > > >> > > > > > > > > it > > > > > >> > > > > > > > > > > > > should > > > > > >> > > > > > > > > > > > > > be treated as private by developers. I > > don't > > > > > >> think > > > > > >> > > the > > > > > >> > > > > fact > > > > > >> > > > > > > it's > > > > > >> > > > > > > > > > > > > > omitted from our guidelines changes > > > > perception > > > > > >> of > > > > > >> > it. > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > Em ter., 23 de jun. de 2020 às 01:15, > > > > Bharath > > > > > >> > > > > Vissapragada > > > > > >> > > > > > < > > > > > >> > > > > > > > > > > > > > [email protected]> escreveu: > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > Sorry, I should've been clearer. > It's > > the > > > > > >> former. > > > > > >> > > My > > > > > >> > > > > > point > > > > > >> > > > > > > is, > > > > > >> > > > > > > > > > any > > > > > >> > > > > > > > > > > > > method > > > > > >> > > > > > > > > > > > > > > tagged with @VisibleForTesting is > only > > > > > >> intended > > > > > >> > for > > > > > >> > > > > > testing > > > > > >> > > > > > > > > > > purposes > > > > > >> > > > > > > > > > > > > and > > > > > >> > > > > > > > > > > > > > > should _not_ be considered public, > its > > > > > >> visibility > > > > > >> > > > scope > > > > > >> > > > > > is > > > > > >> > > > > > > > > wider > > > > > >> > > > > > > > > > > than > > > > > >> > > > > > > > > > > > > > > necessary only because it was needed > > by > > > > some > > > > > >> test > > > > > >> > > > > method. > > > > > >> > > > > > > > > That's > > > > > >> > > > > > > > > > > how > > > > > >> > > > > > > > > > > > > I'd > > > > > >> > > > > > > > > > > > > > > interpret it (Actually, that's what > I > > > > > thought > > > > > >> you > > > > > >> > > > > meant, > > > > > >> > > > > > > now > > > > > >> > > > > > > > > I'm > > > > > >> > > > > > > > > > > > > confused > > > > > >> > > > > > > > > > > > > > > :-)). > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > On Mon, Jun 22, 2020 at 4:02 PM Nick > > > > > Dimiduk < > > > > > >> > > > > > > > > > [email protected]> > > > > > >> > > > > > > > > > > > > > wrote: > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > On Mon, Jun 22, 2020 at 3:45 PM > > Bharath > > > > > >> > > > Vissapragada > > > > > >> > > > > < > > > > > >> > > > > > > > > > > > > > > [email protected]> > > > > > >> > > > > > > > > > > > > > > > wrote: > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > I share the same opinion. Infact > > > > hadoop > > > > > >> (from > > > > > >> > > > which > > > > > >> > > > > > our > > > > > >> > > > > > > > > > > > annotations > > > > > >> > > > > > > > > > > > > > are > > > > > >> > > > > > > > > > > > > > > > > derived I believe), talks about > > this, > > > > > >> "Also, > > > > > >> > > > > certain > > > > > >> > > > > > > APIs > > > > > >> > > > > > > > > are > > > > > >> > > > > > > > > > > > > > annotated > > > > > >> > > > > > > > > > > > > > > > as > > > > > >> > > > > > > > > > > > > > > > > @VisibleForTesting (from > > > > > com.google.common > > > > > >> > > > > > > > > > > > > > > > .annotations.VisibleForTesting) > > > > > >> > > > > > > > > > > > > > > > > - these are meant to be used > > strictly > > > > > for > > > > > >> > unit > > > > > >> > > > > tests > > > > > >> > > > > > > and > > > > > >> > > > > > > > > > should > > > > > >> > > > > > > > > > > > be > > > > > >> > > > > > > > > > > > > > > > treated > > > > > >> > > > > > > > > > > > > > > > > as “Private” APIs." > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > > > > > > > > https://hadoop.apache.org/docs/r3.1.2/hadoop-project-dist/hadoop-common/InterfaceClassification.html > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > Sorry Bharath, I don't follow. Are > > you > > > > > >> saying > > > > > >> > "I > > > > > >> > > > > share > > > > > >> > > > > > > the > > > > > >> > > > > > > > > > > opinion > > > > > >> > > > > > > > > > > > > that > > > > > >> > > > > > > > > > > > > > > the > > > > > >> > > > > > > > > > > > > > > > VisibleForTesting annotation > should > > be > > > > > >> > considered > > > > > >> > > > as > > > > > >> > > > > > > > > defining a > > > > > >> > > > > > > > > > > > > method > > > > > >> > > > > > > > > > > > > > as > > > > > >> > > > > > > > > > > > > > > > IA.Private," and this is an > omission > > > > from > > > > > >> our > > > > > >> > > > > community > > > > > >> > > > > > > > > > > guidelines > > > > > >> > > > > > > > > > > > > > > > document? Or are you saying "no, > it > > does > > > > > not > > > > > >> > > count > > > > > >> > > > as > > > > > >> > > > > > an > > > > > >> > > > > > > > > > > interface > > > > > >> > > > > > > > > > > > > > > audience > > > > > >> > > > > > > > > > > > > > > > marker," and we are obliged to > treat > > > > > methods > > > > > >> > such > > > > > >> > > > as > > > > > >> > > > > in > > > > > >> > > > > > > this > > > > > >> > > > > > > > > > > > example > > > > > >> > > > > > > > > > > > > as > > > > > >> > > > > > > > > > > > > > > > public API? > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > Thanks, > > > > > >> > > > > > > > > > > > > > > > Nick > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > On Mon, Jun 22, 2020 at 10:15 AM > > Sean > > > > > >> Busbey < > > > > > >> > > > > > > > > > [email protected]> > > > > > >> > > > > > > > > > > > > > wrote: > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > Yeah I would say no as well. > We > > > > should > > > > > >> make > > > > > >> > > > clear > > > > > >> > > > > > on > > > > > >> > > > > > > our > > > > > >> > > > > > > > > > dev > > > > > >> > > > > > > > > > > > > guide > > > > > >> > > > > > > > > > > > > > > that > > > > > >> > > > > > > > > > > > > > > > > you > > > > > >> > > > > > > > > > > > > > > > > > also should be marking those > > things > > > > > >> with an > > > > > >> > > > > > Interface > > > > > >> > > > > > > > > > > Audience > > > > > >> > > > > > > > > > > > > > > marking > > > > > >> > > > > > > > > > > > > > > > if > > > > > >> > > > > > > > > > > > > > > > > > you don't intend them to be at > > the > > > > > >> > downstream > > > > > >> > > > API > > > > > >> > > > > > > > > > visibility > > > > > >> > > > > > > > > > > of > > > > > >> > > > > > > > > > > > > the > > > > > >> > > > > > > > > > > > > > > > > parent > > > > > >> > > > > > > > > > > > > > > > > > class. > > > > > >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > (IIRC we also use > > VisibleForTesting > > > > in > > > > > >> > > > IA.Private > > > > > >> > > > > > > classes > > > > > >> > > > > > > > > > to > > > > > >> > > > > > > > > > > > > > > > proactively > > > > > >> > > > > > > > > > > > > > > > > > explain why some internal > > looking > > > > > >> member is > > > > > >> > > at > > > > > >> > > > a > > > > > >> > > > > > > wider > > > > > >> > > > > > > > > Java > > > > > >> > > > > > > > > > > > > access > > > > > >> > > > > > > > > > > > > > > > > scope.) > > > > > >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > On Mon, Jun 22, 2020, 11:39 > Nick > > > > > >> Dimiduk < > > > > > >> > > > > > > > > > > [email protected]> > > > > > >> > > > > > > > > > > > > > > wrote: > > > > > >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > Hello, > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > This came up over on the > > 2.3.0RC0 > > > > > >> thread, > > > > > >> > > so > > > > > >> > > > > > let's > > > > > >> > > > > > > open > > > > > >> > > > > > > > > > it > > > > > >> > > > > > > > > > > > for > > > > > >> > > > > > > > > > > > > > > proper > > > > > >> > > > > > > > > > > > > > > > > > > discussion. In that context, > > we > > > > > >> observe > > > > > >> > > > method > > > > > >> > > > > > > > > signature > > > > > >> > > > > > > > > > > > > changes > > > > > >> > > > > > > > > > > > > > > to a > > > > > >> > > > > > > > > > > > > > > > > > > method marked with the Guava > > > > > >> > > > VisibleForTesting > > > > > >> > > > > > > > > > annotation. > > > > > >> > > > > > > > > > > > The > > > > > >> > > > > > > > > > > > > > > method > > > > > >> > > > > > > > > > > > > > > > > is > > > > > >> > > > > > > > > > > > > > > > > > a > > > > > >> > > > > > > > > > > > > > > > > > > protected method on a > > IA.Public > > > > > class. > > > > > >> > > There > > > > > >> > > > is > > > > > >> > > > > > no > > > > > >> > > > > > > > > > > > method-level > > > > > >> > > > > > > > > > > > > > IA > > > > > >> > > > > > > > > > > > > > > > > > > annotation. > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > Do we consider the > > > > VisibleForTesting > > > > > >> > > > annotation > > > > > >> > > > > > as > > > > > >> > > > > > > a > > > > > >> > > > > > > > > > > > specifier > > > > > >> > > > > > > > > > > > > > for > > > > > >> > > > > > > > > > > > > > > > our > > > > > >> > > > > > > > > > > > > > > > > > > compatibility guidelines? > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > I am of the opinion that no, > > it is > > > > > >> not an > > > > > >> > > > > > > > > > InterfaceAudience > > > > > >> > > > > > > > > > > > > > > > annotation, > > > > > >> > > > > > > > > > > > > > > > > > and > > > > > >> > > > > > > > > > > > > > > > > > > so it is not applicable for > > > > defining > > > > > >> our > > > > > >> > > > public > > > > > >> > > > > > > API. > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > What do you think? > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > Thanks, > > > > > >> > > > > > > > > > > > > > > > > > > Nick > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > -- > > > > > >> > > > > > > > > Best regards, > > > > > >> > > > > > > > > Andrew > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > Words like orphans lost among the crosstalk, > > meaning > > > > > torn > > > > > >> > from > > > > > >> > > > > > truth's > > > > > >> > > > > > > > > decrepit hands > > > > > >> > > > > > > > > - A23, Crosstalk > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > -- > > > > > >> > > > > Best regards, > > > > > >> > > > > Andrew > > > > > >> > > > > > > > > > >> > > > > Words like orphans lost among the crosstalk, meaning > torn > > from > > > > > >> > truth's > > > > > >> > > > > decrepit hands > > > > > >> > > > > - A23, Crosstalk > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > > >> > > -- > > > > > >> > > Best regards, > > > > > >> > > Andrew > > > > > >> > > > > > > > >> > > Words like orphans lost among the crosstalk, meaning torn > from > > > > > truth's > > > > > >> > > decrepit hands > > > > > >> > > - A23, Crosstalk > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > > >> -- > > > > > >> Best regards, > > > > > >> Andrew > > > > > >> > > > > > >> Words like orphans lost among the crosstalk, meaning torn from > > truth's > > > > > >> decrepit hands > > > > > >> - A23, Crosstalk > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > -- > > > Best regards, > > > Andrew > > > > > > Words like orphans lost among the crosstalk, meaning torn from truth's > > > decrepit hands > > > - A23, Crosstalk > > > > > >
