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 <ndimi...@apache.org> 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 < > wellington.chevre...@gmail.com> 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) < > palomino...@gmail.com > > > > > 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 <wellington.chevre...@gmail.com> 于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 < > > > > bhara...@apache.org> 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 <ndimi...@apache.org> > > > > wrote: > > > > > > > > > > > On Mon, Jun 22, 2020 at 3:45 PM Bharath Vissapragada < > > > > > bhara...@apache.org> > > > > > > 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 <bus...@apache.org> > > > > 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 < > ndimi...@apache.org> > > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
