> > Could you point me some document / ML thread this was explicitly decided > in if you know of anything like that? It would be great if there was some > solid guidance on this.
I am seeing it the other way around. Using Deprecated annotations make sense only if something is part of a public interface/API. Maintaining a public API represent a significant work and put some constraints on further evolution. By default most of the code of C* should be considered as internal and we should be able to modify it without going through a deprecation phase. One problem that we have is that we have never been clear, outside of some obvious stuff, about what code should be consider as APIs that need to go through a deprecation phase. Le ven. 13 oct. 2023 à 13:13, Miklosovic, Stefan via dev < dev@cassandra.apache.org> a écrit : > OK. That is definitely something to mention when we will approach the > second phase where we decide what do with it but I humbly think we are not > there yet. > > Could you point me some document / ML thread this was explicitly decided > in if you know of anything like that? It would be great if there was some > solid guidance on this. > > ________________________________________ > From: Benjamin Lerer <ble...@apache.org> > Sent: Friday, October 13, 2023 13:07 > 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 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<mailto: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<mailto:b.le...@gmail.com>> > Sent: Friday, October 13, 2023 12:19 > 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. > > > > 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><mailto: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 > ><mailto: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%7C9df7469de0f24c1dc7cd08dbcbdca30c%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327920731484523%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=JuLp79MXezkhKeOIChnaUYqaePB8zWMWe4VKGAlxZ%2Fg%3D&reserved=0 > >< > 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 > < > 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%7C9df7469de0f24c1dc7cd08dbcbdca30c%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327920731484523%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=JuLp79MXezkhKeOIChnaUYqaePB8zWMWe4VKGAlxZ%2Fg%3D&reserved=0 > >> > > > > > > ________________________________________ > > > From: Francisco Guerrero <fran...@apache.org<mailto:fran...@apache.org > ><mailto:fran...@apache.org<mailto:fran...@apache.org>>> > > > Sent: Tuesday, October 10, 2023 23:34 > > > To: dev@cassandra.apache.org<mailto:dev@cassandra.apache.org><mailto: > 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%7C9df7469de0f24c1dc7cd08dbcbdca30c%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327920731484523%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=JuLp79MXezkhKeOIChnaUYqaePB8zWMWe4VKGAlxZ%2Fg%3D&reserved=0 > >< > 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 > < > 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%7C9df7469de0f24c1dc7cd08dbcbdca30c%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327920731484523%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=JuLp79MXezkhKeOIChnaUYqaePB8zWMWe4VKGAlxZ%2Fg%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%7C9df7469de0f24c1dc7cd08dbcbdca30c%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327920731484523%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=gplySUutOd3gOOZsW5Mvfr1FtdttDkEBkJE6RQRtroM%3D&reserved=0 > >< > 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 > < > 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%7C9df7469de0f24c1dc7cd08dbcbdca30c%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327920731484523%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=gplySUutOd3gOOZsW5Mvfr1FtdttDkEBkJE6RQRtroM%3D&reserved=0 > >> > > > > > > > > On Tue, 10 Oct 2023 at 14:50, Josh McKenzie <jmcken...@apache.org > <mailto:jmcken...@apache.org><mailto: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 > ><mailto:m...@apache.org<mailto:m...@apache.org>>> > > > > > Sent: Monday, October 9, 2023 10:55 > > > > > To: dev@cassandra.apache.org<mailto:dev@cassandra.apache.org > ><mailto: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. > > > > > > > > > > > > > > > > >
