I’ve been told in the past not to remove public methods in a patch release though.
On Fri, 13 Oct 2023 at 8:03, Benjamin Lerer <ble...@apache.org> wrote: > 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. >> > > > > >> > > > > >> > > > >> > >> >
