Francisco, I agree with your vision of the deprecation comments and actually, I think we should recommend doing it that way for the cases where it is applicable on our code-style page, but when things get to the implementation phase there are some obstacles that are not easy to overcome.
So, adding the MissingDeprecated will emphasize to a developer the need to describe the deprecation reasons in comments, but unfortunately, there is no general pattern that we can enforce for every such description message and/or automatically validate its meaningfulness. There may be no alternative for a deprecated field, or it may simply be marked for deletion, so the pattern is slightly different in this case. Another problem is how to add meaningful comments to the deprecated annotations that we already have in the code, since we can't enforce checkstyle rules only on newly added code. This is a very exhausting process with no 100% guarantee of accuracy - some of the commits don't have a good commit message and require a deep archaeology. All of the above led me to the following which is pretty easy to achieve and improves the code quality: /** @deprecated See CASSANDRA-6504 */ @Deprecated(since = "2.1") public Integer concurrent_replicates = null; On Wed, 11 Oct 2023 at 09:51, Miklosovic, Stefan <stefan.mikloso...@netapp.com> wrote: > > Here (1) it supports check of both Javadoc and annotation at the same time so > what you want is possible. What is not possible is to checkstyle the > _content_ of deprecated Javadoc nor any format of it. I think that ensuring > the presence of both annotation and Javadoc comment is just enough. > > (1) > https://checkstyle.sourceforge.io/apidocs/com/puppycrawl/tools/checkstyle/checks/annotation/MissingDeprecatedCheck.html > > ________________________________________ > From: Francisco Guerrero <fran...@apache.org> > Sent: Tuesday, October 10, 2023 23:34 > To: dev@cassandra.apache.org > Subject: Re: [DISCUSS] putting versions into Deprecated annotations > > NetApp Security WARNING: This is an external email. Do not click links or > open attachments unless you recognize the sender and know the content is safe. > > > > > To me this seems insufficient. As a developer, I'd like to see what the > alternative is when reading the javadoc without having to go to Jira. > > What I would prefer is to know what the alternative is and how to use it. For > example: > > /** @deprecated Use {@link #alternative} instead. See CASSANDRA-6504 */ > @Deprecated(since = "2.1") > public Integer concurrent_replicates = null; > > I am not sure if checkstyle can enforce the above, so the mechanisms to > enforce it would still need to be laid out, unless we can easily support > something like the above with checkstyle rules. > > On 2023/10/10 20:34:27 Maxim Muzafarov wrote: > > Hello everyone, > > > > > > I've discussed with Stefan some steps we can take to improve the final > > solution, so the final version might look like this: > > > > /** @deprecated See CASSANDRA-6504 */ > > @Deprecated(since = "2.1") > > public Integer concurrent_replicates = null; > > > > The issue number will be taken from the git blame comment. I doubt I > > can generate and/or create a meaningful comment for every deprecation > > annotation, but providing a link to the issue that was retrieved from > > the git blame is not too big a problem. This also improves the > > visibility. > > > > In addition, we can add two checkstyle rules [1] [2] to ensure that > > any future deprecations will have a "since" element and a JavaDoc > > comment. > > WDYT? > > > > [1] > > https://checkstyle.sourceforge.io/apidocs/com/puppycrawl/tools/checkstyle/checks/annotation/MissingDeprecatedCheck.html > > [2] > > https://checkstyle.org/apidocs/com/puppycrawl/tools/checkstyle/checks/coding/MatchXpathCheck.html > > > > On Tue, 10 Oct 2023 at 14:50, Josh McKenzie <jmcken...@apache.org> wrote: > > > > > > Sounds like we're relitigating the basics of how @Deprecated, forRemoval, > > > since, and javadoc @link all intersect to make deprecation less painful ;) > > > > > > So: > > > > > > Built-in java.lang.Deprecated: required > > > Can use since and forRemoval if you have that info handy and think it'd > > > be useful (would make it a lot easier to grep for things to pull before a > > > major) > > > If it's being replaced by something, you should {@link #} the javadoc for > > > it so people know where to bounce over to > > > > > > I've been leaning pretty heavily on the functionality of point 3 for > > > documenting cross-module implicit dependencies as I come across them > > > lately so that one resonates with me. > > > > > > On Tue, Oct 10, 2023, at 4:38 AM, Miklosovic, Stefan wrote: > > > > > > OK. > > > > > > Let's go with in-built java.lang.Deprecated annotation. If somebody wants > > > to document that in more detail, there are Javadocs as mentioned. Let's > > > just stick with the standard stuff. > > > > > > I will try to implement this for 5.0 (versions since it was deprecated) > > > with my take on what should be removed (forRemoval = true) but that > > > should be definitely cross-checked on review as Mick mentioned. > > > > > > ________________________________________ > > > From: Mick Semb Wever <m...@apache.org> > > > Sent: Monday, October 9, 2023 10:55 > > > To: dev@cassandra.apache.org > > > Subject: Re: [DISCUSS] putting versions into Deprecated annotations > > > > > > NetApp Security WARNING: This is an external email. Do not click links or > > > open attachments unless you recognize the sender and know the content is > > > safe. > > > > > > > > > > > > Tangential question to this is if everything we deprecated is eligible > > > for removal? In other words, are there any cases when forRemoval would be > > > false? Could you elaborate on that and give such examples or do you all > > > think that everything which is deprecated will be eventually removed? > > > > > > > > > Removal cannot be default. This came up in the subtickets of > > > CASSANDRA-18306. > > > > > > I suggest that adding " forRemoval = true" and the later actual removal > > > of the code both require broader consensus. I'm open to that being on > > > the ticket or needing a thread on the ML. Small stuff, common sense says > > > on the ticket is enough, but a few folk have already stated that > > > deprecated code that has minimal maintenance overhead should not be > > > removed. > > > > > > > >
