Hi, all
Let’s have look at Checkpointed interface below. It declared deprecated but have no detail for why, when and how replace this function. It’s a big trouble for the users. @Deprecated @PublicEvolving public interface Checkpointed<T extends Serializable> extends CheckpointedRestoring<T> { I think we should have more detail: when give up, who replace it, why deprecated. For Java code, add detail deprecated reason in code annotations. For Scala code, replace Java annotation @Deprecated(,,) with Scala annotation @deprecated, such as @deprecated(message = "the reason", since = "when fully give up”) Add this rule to customized checkstyle plugin of maven and SBT. Best regard -Jinkui Shi |
I agree on this one.
Whenever we deprecate a method or a feature we should add a comment that explains the new API or why the feature was removed without replacement. Enforcing this information through checkstyle makes sense as well, IMO. Cheers, Fabian 2016-11-23 4:42 GMT+01:00 sjk <[hidden email]>: > Hi, all > > Let’s have look at Checkpointed interface below. It declared deprecated > but have no detail for why, when and how replace this function. It’s a big > trouble for the users. > > @Deprecated > @PublicEvolving > public interface Checkpointed<T extends Serializable> extends > CheckpointedRestoring<T> { > > > I think we should have more detail: when give up, who replace it, why > deprecated. > > For Java code, add detail deprecated reason in code annotations. > For Scala code, replace Java annotation @Deprecated(,,) with Scala > annotation @deprecated, such as > @deprecated(message = "the reason", since = "when fully give up”) > > Add this rule to customized checkstyle plugin of maven and SBT. > > Best regard > -Jinkui Shi |
+1 for your proposal.
Cheers, Till On Wed, Nov 23, 2016 at 9:33 AM, Fabian Hueske <[hidden email]> wrote: > I agree on this one. > Whenever we deprecate a method or a feature we should add a comment that > explains the new API or why the feature was removed without replacement. > > Enforcing this information through checkstyle makes sense as well, IMO. > > Cheers, Fabian > > 2016-11-23 4:42 GMT+01:00 sjk <[hidden email]>: > > > Hi, all > > > > Let’s have look at Checkpointed interface below. It declared deprecated > > but have no detail for why, when and how replace this function. It’s a > big > > trouble for the users. > > > > @Deprecated > > @PublicEvolving > > public interface Checkpointed<T extends Serializable> extends > > CheckpointedRestoring<T> { > > > > > > I think we should have more detail: when give up, who replace it, why > > deprecated. > > > > For Java code, add detail deprecated reason in code annotations. > > For Scala code, replace Java annotation @Deprecated(,,) with Scala > > annotation @deprecated, such as > > @deprecated(message = "the reason", since = "when fully give up”) > > > > Add this rule to customized checkstyle plugin of maven and SBT. > > > > Best regard > > -Jinkui Shi > |
+1 That sounds excellent.
On Wed, 23 Nov 2016 at 11:04 Till Rohrmann <[hidden email]> wrote: > +1 for your proposal. > > Cheers, > Till > > On Wed, Nov 23, 2016 at 9:33 AM, Fabian Hueske <[hidden email]> wrote: > > > I agree on this one. > > Whenever we deprecate a method or a feature we should add a comment that > > explains the new API or why the feature was removed without replacement. > > > > Enforcing this information through checkstyle makes sense as well, IMO. > > > > Cheers, Fabian > > > > 2016-11-23 4:42 GMT+01:00 sjk <[hidden email]>: > > > > > Hi, all > > > > > > Let’s have look at Checkpointed interface below. It declared deprecated > > > but have no detail for why, when and how replace this function. It’s a > > big > > > trouble for the users. > > > > > > @Deprecated > > > @PublicEvolving > > > public interface Checkpointed<T extends Serializable> extends > > > CheckpointedRestoring<T> { > > > > > > > > > I think we should have more detail: when give up, who replace it, why > > > deprecated. > > > > > > For Java code, add detail deprecated reason in code annotations. > > > For Scala code, replace Java annotation @Deprecated(,,) with Scala > > > annotation @deprecated, such as > > > @deprecated(message = "the reason", since = "when fully give up”) > > > > > > Add this rule to customized checkstyle plugin of maven and SBT. > > > > > > Best regard > > > -Jinkui Shi > > > |
+1 and we should apply the same to all deprecated interfaces/abstract classes.
> On Nov 23, 2016, at 11:13 AM, Aljoscha Krettek <[hidden email]> wrote: > > +1 That sounds excellent. > > On Wed, 23 Nov 2016 at 11:04 Till Rohrmann <[hidden email]> wrote: > >> +1 for your proposal. >> >> Cheers, >> Till >> >> On Wed, Nov 23, 2016 at 9:33 AM, Fabian Hueske <[hidden email]> wrote: >> >>> I agree on this one. >>> Whenever we deprecate a method or a feature we should add a comment that >>> explains the new API or why the feature was removed without replacement. >>> >>> Enforcing this information through checkstyle makes sense as well, IMO. >>> >>> Cheers, Fabian >>> >>> 2016-11-23 4:42 GMT+01:00 sjk <[hidden email]>: >>> >>>> Hi, all >>>> >>>> Let’s have look at Checkpointed interface below. It declared deprecated >>>> but have no detail for why, when and how replace this function. It’s a >>> big >>>> trouble for the users. >>>> >>>> @Deprecated >>>> @PublicEvolving >>>> public interface Checkpointed<T extends Serializable> extends >>>> CheckpointedRestoring<T> { >>>> >>>> >>>> I think we should have more detail: when give up, who replace it, why >>>> deprecated. >>>> >>>> For Java code, add detail deprecated reason in code annotations. >>>> For Scala code, replace Java annotation @Deprecated(,,) with Scala >>>> annotation @deprecated, such as >>>> @deprecated(message = "the reason", since = "when fully give up”) >>>> >>>> Add this rule to customized checkstyle plugin of maven and SBT. >>>> >>>> Best regard >>>> -Jinkui Shi >>> >> |
+1
This should always be the norm, especially for user-facing code. While we are at it, perhaps when someone deprecates functionality the new alternative should also be replaced right away. E.g. Checkpointed is deprecated but all state management tests are actually using this alternative. cheers Paris > On 23 Nov 2016, at 11:21, Kostas Kloudas <[hidden email]> wrote: > > +1 and we should apply the same to all deprecated interfaces/abstract classes. > >> On Nov 23, 2016, at 11:13 AM, Aljoscha Krettek <[hidden email]> wrote: >> >> +1 That sounds excellent. >> >> On Wed, 23 Nov 2016 at 11:04 Till Rohrmann <[hidden email]> wrote: >> >>> +1 for your proposal. >>> >>> Cheers, >>> Till >>> >>> On Wed, Nov 23, 2016 at 9:33 AM, Fabian Hueske <[hidden email]> wrote: >>> >>>> I agree on this one. >>>> Whenever we deprecate a method or a feature we should add a comment that >>>> explains the new API or why the feature was removed without replacement. >>>> >>>> Enforcing this information through checkstyle makes sense as well, IMO. >>>> >>>> Cheers, Fabian >>>> >>>> 2016-11-23 4:42 GMT+01:00 sjk <[hidden email]>: >>>> >>>>> Hi, all >>>>> >>>>> Let’s have look at Checkpointed interface below. It declared deprecated >>>>> but have no detail for why, when and how replace this function. It’s a >>>> big >>>>> trouble for the users. >>>>> >>>>> @Deprecated >>>>> @PublicEvolving >>>>> public interface Checkpointed<T extends Serializable> extends >>>>> CheckpointedRestoring<T> { >>>>> >>>>> >>>>> I think we should have more detail: when give up, who replace it, why >>>>> deprecated. >>>>> >>>>> For Java code, add detail deprecated reason in code annotations. >>>>> For Scala code, replace Java annotation @Deprecated(,,) with Scala >>>>> annotation @deprecated, such as >>>>> @deprecated(message = "the reason", since = "when fully give up”) >>>>> >>>>> Add this rule to customized checkstyle plugin of maven and SBT. >>>>> >>>>> Best regard >>>>> -Jinkui Shi >>>> >>> > |
+1
I think we actually had the same discussion already a while back. Let's bring it back to everyone's awareness! On Wed, Nov 23, 2016 at 12:09 PM, Paris Carbone <[hidden email]> wrote: > +1 > > This should always be the norm, especially for user-facing code. > > While we are at it, perhaps when someone deprecates functionality the new > alternative should also be replaced right away. > E.g. Checkpointed is deprecated but all state management tests are > actually using this alternative. > > cheers > Paris > > > > On 23 Nov 2016, at 11:21, Kostas Kloudas <[hidden email]> > wrote: > > > > +1 and we should apply the same to all deprecated interfaces/abstract > classes. > > > >> On Nov 23, 2016, at 11:13 AM, Aljoscha Krettek <[hidden email]> > wrote: > >> > >> +1 That sounds excellent. > >> > >> On Wed, 23 Nov 2016 at 11:04 Till Rohrmann <[hidden email]> > wrote: > >> > >>> +1 for your proposal. > >>> > >>> Cheers, > >>> Till > >>> > >>> On Wed, Nov 23, 2016 at 9:33 AM, Fabian Hueske <[hidden email]> > wrote: > >>> > >>>> I agree on this one. > >>>> Whenever we deprecate a method or a feature we should add a comment > that > >>>> explains the new API or why the feature was removed without > replacement. > >>>> > >>>> Enforcing this information through checkstyle makes sense as well, > IMO. > >>>> > >>>> Cheers, Fabian > >>>> > >>>> 2016-11-23 4:42 GMT+01:00 sjk <[hidden email]>: > >>>> > >>>>> Hi, all > >>>>> > >>>>> Let’s have look at Checkpointed interface below. It declared > deprecated > >>>>> but have no detail for why, when and how replace this function. It’s > a > >>>> big > >>>>> trouble for the users. > >>>>> > >>>>> @Deprecated > >>>>> @PublicEvolving > >>>>> public interface Checkpointed<T extends Serializable> extends > >>>>> CheckpointedRestoring<T> { > >>>>> > >>>>> > >>>>> I think we should have more detail: when give up, who replace it, why > >>>>> deprecated. > >>>>> > >>>>> For Java code, add detail deprecated reason in code annotations. > >>>>> For Scala code, replace Java annotation @Deprecated(,,) with Scala > >>>>> annotation @deprecated, such as > >>>>> @deprecated(message = "the reason", since = "when fully give up”) > >>>>> > >>>>> Add this rule to customized checkstyle plugin of maven and SBT. > >>>>> > >>>>> Best regard > >>>>> -Jinkui Shi > >>>> > >>> > > > > |
There is a related checkstyle rule:
http://checkstyle.sourceforge.net/apidocs/com/puppycrawl/tools/checkstyle/checks/annotation/MissingDeprecatedCheck.html Added a JIRA for adding it here: https://issues.apache.org/jira/browse/FLINK-6127 We actually wrote this down in our hidden Wiki at https://cwiki.apache.org/confluence/display/FLINK/Best+Practices+and+Lessons+Learned > - Always add comments when you deprecate something > * Add a @Deprecated annotation to the JavaDocs explaining why it was deprecated (removed, replaced, etc.) > * Create issues with target release version for deleting deprecated interfaces > * https://docs.oracle.com/javase/1.5.0/docs/guide/javadoc/deprecation/deprecation.html On Mon, Mar 20, 2017 at 11:33 AM, Stephan Ewen <[hidden email]> wrote: > +1 > > I think we actually had the same discussion already a while back. Let's > bring it back to everyone's awareness! > > > > On Wed, Nov 23, 2016 at 12:09 PM, Paris Carbone <[hidden email]> wrote: > >> +1 >> >> This should always be the norm, especially for user-facing code. >> >> While we are at it, perhaps when someone deprecates functionality the new >> alternative should also be replaced right away. >> E.g. Checkpointed is deprecated but all state management tests are >> actually using this alternative. >> >> cheers >> Paris >> >> >> > On 23 Nov 2016, at 11:21, Kostas Kloudas <[hidden email]> >> wrote: >> > >> > +1 and we should apply the same to all deprecated interfaces/abstract >> classes. >> > >> >> On Nov 23, 2016, at 11:13 AM, Aljoscha Krettek <[hidden email]> >> wrote: >> >> >> >> +1 That sounds excellent. >> >> >> >> On Wed, 23 Nov 2016 at 11:04 Till Rohrmann <[hidden email]> >> wrote: >> >> >> >>> +1 for your proposal. >> >>> >> >>> Cheers, >> >>> Till >> >>> >> >>> On Wed, Nov 23, 2016 at 9:33 AM, Fabian Hueske <[hidden email]> >> wrote: >> >>> >> >>>> I agree on this one. >> >>>> Whenever we deprecate a method or a feature we should add a comment >> that >> >>>> explains the new API or why the feature was removed without >> replacement. >> >>>> >> >>>> Enforcing this information through checkstyle makes sense as well, >> IMO. >> >>>> >> >>>> Cheers, Fabian >> >>>> >> >>>> 2016-11-23 4:42 GMT+01:00 sjk <[hidden email]>: >> >>>> >> >>>>> Hi, all >> >>>>> >> >>>>> Let’s have look at Checkpointed interface below. It declared >> deprecated >> >>>>> but have no detail for why, when and how replace this function. It’s >> a >> >>>> big >> >>>>> trouble for the users. >> >>>>> >> >>>>> @Deprecated >> >>>>> @PublicEvolving >> >>>>> public interface Checkpointed<T extends Serializable> extends >> >>>>> CheckpointedRestoring<T> { >> >>>>> >> >>>>> >> >>>>> I think we should have more detail: when give up, who replace it, why >> >>>>> deprecated. >> >>>>> >> >>>>> For Java code, add detail deprecated reason in code annotations. >> >>>>> For Scala code, replace Java annotation @Deprecated(,,) with Scala >> >>>>> annotation @deprecated, such as >> >>>>> @deprecated(message = "the reason", since = "when fully give up”) >> >>>>> >> >>>>> Add this rule to customized checkstyle plugin of maven and SBT. >> >>>>> >> >>>>> Best regard >> >>>>> -Jinkui Shi >> >>>> >> >>> >> > >> >> |
Free forum by Nabble | Edit this page |