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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
