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