I was asking because outside of configuration parameters and JMX calls, the approach as far as I remember was to just change things without using an annotation.
Le ven. 13 oct. 2023 à 12:45, Miklosovic, Stefan via dev < dev@cassandra.apache.org> a écrit : > Hi Benjamin, > > in other words, anything we have @Deprecated annotation on top of (or > anything you want to annotate with it). Does it help with the explanation? > > For the initial phase, I plan to just put "since" everywhere (into every > already existing @Deprecated annotation) and we leave out "forRemoval" in > Deprecated annotation for now as that is quite tricky to get right. > > I am confused what is considered to be removed and what we keep there for > ever even it is deprecated (referring to what Mick said in this thread that > forRemoval can not be by default true). After we map what technical debt we > have, we can summarize this and I bring it to the ML again for further > discussion what to actually remove and when. > > Regards > > ________________________________________ > From: Benjamin Lerer <b.le...@gmail.com> > Sent: Friday, October 13, 2023 12:19 > 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. > > > > I am a bit confused by the starting point of this discussion: "When we > deprecate APIs / methods" > What are we exactly calling APIs/methods? It is really unclear to me what > we are talking about here. > > Le jeu. 12 oct. 2023 à 02:38, Francisco Guerrero <fran...@apache.org > <mailto:fran...@apache.org>> a écrit : > > > On 2023/10/11 16:59:35 Maxim Muzafarov wrote: > > 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. > > Yeah, I agree that this should be recommended rather than enforced via > some checkstyle rule. However, reviewers should be aware of this > recommendation in the code-style page. > > > > > 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. > > > +1 for adding the MissingDeprecated rule > > > 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. > > Not aiming for 100% accuracy, but more on code style agreement. > > > 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<mailto: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 > < > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcheckstyle.sourceforge.io%2Fapidocs%2Fcom%2Fpuppycrawl%2Ftools%2Fcheckstyle%2Fchecks%2Fannotation%2FMissingDeprecatedCheck.html&data=05%7C01%7CStefan.Miklosovic%40netapp.com%7C59fa2b3786ff436c83ba08dbcbd5ece7%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327891917050879%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=8qKu8ob%2BvPdHfUQdkxr5C%2BgkR5iMcUaEqw9a%2FNN276k%3D&reserved=0 > > > > > > > > ________________________________________ > > > From: Francisco Guerrero <fran...@apache.org<mailto:fran...@apache.org > >> > > > Sent: Tuesday, October 10, 2023 23:34 > > > To: dev@cassandra.apache.org<mailto: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 > < > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcheckstyle.sourceforge.io%2Fapidocs%2Fcom%2Fpuppycrawl%2Ftools%2Fcheckstyle%2Fchecks%2Fannotation%2FMissingDeprecatedCheck.html&data=05%7C01%7CStefan.Miklosovic%40netapp.com%7C59fa2b3786ff436c83ba08dbcbd5ece7%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327891917050879%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=8qKu8ob%2BvPdHfUQdkxr5C%2BgkR5iMcUaEqw9a%2FNN276k%3D&reserved=0 > > > > > > [2] > https://checkstyle.org/apidocs/com/puppycrawl/tools/checkstyle/checks/coding/MatchXpathCheck.html > < > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcheckstyle.org%2Fapidocs%2Fcom%2Fpuppycrawl%2Ftools%2Fcheckstyle%2Fchecks%2Fcoding%2FMatchXpathCheck.html&data=05%7C01%7CStefan.Miklosovic%40netapp.com%7C59fa2b3786ff436c83ba08dbcbd5ece7%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327891917050879%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=U6MIohIvPO%2FyGZIHoATZRxwfnvuiyaCtGaBzz0bqw1k%3D&reserved=0 > > > > > > > > > > On Tue, 10 Oct 2023 at 14:50, Josh McKenzie <jmcken...@apache.org > <mailto: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<mailto:m...@apache.org>> > > > > > Sent: Monday, October 9, 2023 10:55 > > > > > To: dev@cassandra.apache.org<mailto: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. > > > > > > > > > > > > > > > > >
