+1. Annotation beats comment in this case, and @VisibleForTesting is more
descriptive than our old anno.
On Mon, Dec 17, 2018 at 3:43 PM Jinmei Liao <jil...@pivotal.io> wrote:
> +1. Yes, the original intention was exactly as @VisibleForTesting.
>
> On Mon, Dec 17, 2018 at 2:45 PM Galen O'Sullivan <gosulli...@pivotal.io>
> wrote:
>
> > Sounds good to me.
> >
> > On Mon, Dec 17, 2018 at 1:04 PM Owen Nichols <onich...@pivotal.io>
> wrote:
> >
> > > Sounds good, I would be happy to +1 a PR for this
> > >
> > > > On Dec 17, 2018, at 12:22 PM, Kirk Lund <kl...@apache.org> wrote:
> > > >
> > > > We have a custom annotation in geode-common called @TestingOnly:
> > > >
> > > >
> > geode-common/src/main/java/org/apache/geode/annotations/TestingOnly.java
> > > >
> > > > This annotation was created while pairing with Michael Feathers and
> the
> > > > intention was to annotate non-private constructors or methods that
> > have a
> > > > widened visibility scope to facilitate testing.
> > > >
> > > > Some developers, however, have interpreted it as meaning that the
> > > > constructor or method cannot be used in the main src code and can
> only
> > be
> > > > used from test src code.
> > > >
> > > > I'd like to propose deleting @TestingOnly and change to
> > > > using @VisibleForTesting which is defined in Guava (which is already
> a
> > > > Geode dependency). The name of the Guava annotation is less ambiguous
> > and
> > > > it's already been adopted for use by additional projects including
> > > AssertJ
> > > > which we use extensively.
> > > >
> > > > The javadocs on VisibleForTesting explains its usage very clearly:
> > > >
> > > > /**
> > > > * Annotates a program element that exists, or is more widely visible
> > > > than otherwise necessary, only
> > > > * for use in test code.
> > > > *
> > > > * @author Johannes Henkel
> > > > */
> > > > @GwtCompatible
> > > > public @interface VisibleForTesting {
> > > > }
> > >
> > >
> >
>
>
> --
> Cheers
>
> Jinmei
>
