You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cassandra.apache.org by "Miklosovic, Stefan" <St...@netapp.com> on 2023/10/06 08:43:05 UTC
[DISCUSS] putting versions into Deprecated annotations
Hi list,
I have a ticket to discuss (1).
When we deprecate APIs / methods etc, what I want to suggest is that we might start to explicitly add the version when that happened. For example, if you deprecated something which goes to 5.0, would you be so nice to do this?
@Deprecated(since = "5.0")
Similarly, that annotation offers one more field - forRemoval, so using it like this:
@Deprecated(since = "5.0", forRemoval = true)
means that this is eligible to be deleted in Cassandra 6.0.
With this information, it is way more comfortable to just "grep" where we are at when it comes to deprecations eligible to be deleted in the next version. Currently, we basically have to go one by one and figure out if it is not old enough to remove. I believe this would bring more transparency into what is planned to be removed and when as well it will be clearly visible what should be removed in the next version and it is not.
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?
(1) https://issues.apache.org/jira/browse/CASSANDRA-18912
Thanks and regards
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by "Miklosovic, Stefan" <St...@netapp.com>.
If we want to use some description (of type String), we would need to introduce a brand new annotation instead of java.lang.Deprecated as that one does not have it.
I am in favor of custom annotation instead of using Javadocs for this kind of technical documentation. An annotation seems to be more succinct, even it is a custom one, rather that using comments for it.
On the other hand, I am not sure how we ensure that developers use this custom annotation instead of the in-built one. java.lang package is not a package to be imported so we can not have a checkstyle rule for it.
________________________________________
From: Francisco Guerrero <fr...@apache.org>
Sent: Saturday, October 7, 2023 0:54
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.
> Might be nice to support a 3rd param that's a String for the reason it's deprecated.
Javadocs offers this natively
/**
* @deprecated Use instance method {@link #newMethod(Param1, Param2...)} instead.
*/
@Deprecated
So we could leverage javadocs for this purpose
On 2023/10/06 11:49:52 Josh McKenzie wrote:
> Might be nice to support a 3rd param that's a String for the reason it's deprecated. i.e. "Replaced by X", "Unmaintained", "Obsolete", "See CASSANDRA-NNNNN", link to a dev ML thread on pony mail, etc. That way if someone comes across it in the codebase they have some context to follow up on if it's the shape of a thing they need w/out having to go full-bore w/git blame and JQL.
>
> On Fri, Oct 6, 2023, at 4:43 AM, Miklosovic, Stefan wrote:
> > Hi list,
> >
> > I have a ticket to discuss (1).
> >
> > When we deprecate APIs / methods etc, what I want to suggest is that we might start to explicitly add the version when that happened. For example, if you deprecated something which goes to 5.0, would you be so nice to do this?
> >
> > @Deprecated(since = "5.0")
> >
> > Similarly, that annotation offers one more field - forRemoval, so using it like this:
> >
> > @Deprecated(since = "5.0", forRemoval = true)
> >
> > means that this is eligible to be deleted in Cassandra 6.0.
> >
> > With this information, it is way more comfortable to just "grep" where we are at when it comes to deprecations eligible to be deleted in the next version. Currently, we basically have to go one by one and figure out if it is not old enough to remove. I believe this would bring more transparency into what is planned to be removed and when as well it will be clearly visible what should be removed in the next version and it is not.
> >
> > 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?
> >
> > (1) https://issues.apache.org/jira/browse/CASSANDRA-18912
> >
> > Thanks and regards
>
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by Francisco Guerrero <fr...@apache.org>.
> Might be nice to support a 3rd param that's a String for the reason it's deprecated.
Javadocs offers this natively
/**
* @deprecated Use instance method {@link #newMethod(Param1, Param2...)} instead.
*/
@Deprecated
So we could leverage javadocs for this purpose
On 2023/10/06 11:49:52 Josh McKenzie wrote:
> Might be nice to support a 3rd param that's a String for the reason it's deprecated. i.e. "Replaced by X", "Unmaintained", "Obsolete", "See CASSANDRA-NNNNN", link to a dev ML thread on pony mail, etc. That way if someone comes across it in the codebase they have some context to follow up on if it's the shape of a thing they need w/out having to go full-bore w/git blame and JQL.
>
> On Fri, Oct 6, 2023, at 4:43 AM, Miklosovic, Stefan wrote:
> > Hi list,
> >
> > I have a ticket to discuss (1).
> >
> > When we deprecate APIs / methods etc, what I want to suggest is that we might start to explicitly add the version when that happened. For example, if you deprecated something which goes to 5.0, would you be so nice to do this?
> >
> > @Deprecated(since = "5.0")
> >
> > Similarly, that annotation offers one more field - forRemoval, so using it like this:
> >
> > @Deprecated(since = "5.0", forRemoval = true)
> >
> > means that this is eligible to be deleted in Cassandra 6.0.
> >
> > With this information, it is way more comfortable to just "grep" where we are at when it comes to deprecations eligible to be deleted in the next version. Currently, we basically have to go one by one and figure out if it is not old enough to remove. I believe this would bring more transparency into what is planned to be removed and when as well it will be clearly visible what should be removed in the next version and it is not.
> >
> > 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?
> >
> > (1) https://issues.apache.org/jira/browse/CASSANDRA-18912
> >
> > Thanks and regards
>
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by Doug Rohrer <dr...@apple.com>.
+1 on reason string, especially some way to indicate what replaces a method if it’s being moved into some other class/new method with more parameters/etc. I’ve found lots of cases (in code bases in general, not C* in particular) where something is marked as Deprecated but there’s no mention of a replacement even when there is one.
As someone who has spent a bunch of time using parts of Cassandra as a library, this would be hugely beneficial, but it would also clearly be useful for maintainers of the core codebase.
Doug
> On Oct 6, 2023, at 7:49 AM, Josh McKenzie <jm...@apache.org> wrote:
>
> Might be nice to support a 3rd param that's a String for the reason it's deprecated. i.e. "Replaced by X", "Unmaintained", "Obsolete", "See CASSANDRA-NNNNN", link to a dev ML thread on pony mail, etc. That way if someone comes across it in the codebase they have some context to follow up on if it's the shape of a thing they need w/out having to go full-bore w/git blame and JQL.
>
> On Fri, Oct 6, 2023, at 4:43 AM, Miklosovic, Stefan wrote:
>> Hi list,
>>
>> I have a ticket to discuss (1).
>>
>> When we deprecate APIs / methods etc, what I want to suggest is that we might start to explicitly add the version when that happened. For example, if you deprecated something which goes to 5.0, would you be so nice to do this?
>>
>> @Deprecated(since = "5.0")
>>
>> Similarly, that annotation offers one more field - forRemoval, so using it like this:
>>
>> @Deprecated(since = "5.0", forRemoval = true)
>>
>> means that this is eligible to be deleted in Cassandra 6.0.
>>
>> With this information, it is way more comfortable to just "grep" where we are at when it comes to deprecations eligible to be deleted in the next version. Currently, we basically have to go one by one and figure out if it is not old enough to remove. I believe this would bring more transparency into what is planned to be removed and when as well it will be clearly visible what should be removed in the next version and it is not.
>>
>> 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?
>>
>> (1) https://issues.apache.org/jira/browse/CASSANDRA-18912
>>
>> Thanks and regards
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by Josh McKenzie <jm...@apache.org>.
Might be nice to support a 3rd param that's a String for the reason it's deprecated. i.e. "Replaced by X", "Unmaintained", "Obsolete", "See CASSANDRA-NNNNN", link to a dev ML thread on pony mail, etc. That way if someone comes across it in the codebase they have some context to follow up on if it's the shape of a thing they need w/out having to go full-bore w/git blame and JQL.
On Fri, Oct 6, 2023, at 4:43 AM, Miklosovic, Stefan wrote:
> Hi list,
>
> I have a ticket to discuss (1).
>
> When we deprecate APIs / methods etc, what I want to suggest is that we might start to explicitly add the version when that happened. For example, if you deprecated something which goes to 5.0, would you be so nice to do this?
>
> @Deprecated(since = "5.0")
>
> Similarly, that annotation offers one more field - forRemoval, so using it like this:
>
> @Deprecated(since = "5.0", forRemoval = true)
>
> means that this is eligible to be deleted in Cassandra 6.0.
>
> With this information, it is way more comfortable to just "grep" where we are at when it comes to deprecations eligible to be deleted in the next version. Currently, we basically have to go one by one and figure out if it is not old enough to remove. I believe this would bring more transparency into what is planned to be removed and when as well it will be clearly visible what should be removed in the next version and it is not.
>
> 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?
>
> (1) https://issues.apache.org/jira/browse/CASSANDRA-18912
>
> Thanks and regards
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by Maxim Muzafarov <mm...@apache.org>.
I think the source code can describe the intention better than words :-)
The link to our Code Style with a discussion "summary":
https://github.com/apache/cassandra-website/pull/245/files
The link to the pull request with the proposed changes (only "since"
added and description):
https://github.com/apache/cassandra/pull/2801/files
On Fri, 13 Oct 2023 at 14:45, Benjamin Lerer <b....@gmail.com> wrote:
>
> Ok, thanks Stefan I understand the context better now. Looking at the PR.
> Some make sense also for serialization reasons but some make no sense to me.
>
>
> Le ven. 13 oct. 2023 à 14:26, Benjamin Lerer <b....@gmail.com> a écrit :
>>>
>>> I’ve been told in the past not to remove public methods in a patch release though.
>>
>>
>> Then I am curious to get the rationale behind that. If some piece of code is not used anymore then simplifying the code is the best thing to do. It makes maintenance easier and avoids mistakes.
>> Le ven. 13 oct. 2023 à 14:11, Miklosovic, Stefan via dev <de...@cassandra.apache.org> a écrit :
>>>
>>> Maybe for better understanding what we talk about, there is the PR which implements the changes suggested here (1)
>>>
>>> It is clear that @Deprecated is not used exclusively on JMX / Configuration but we use it internally as well. This is a very delicate topic and we need to go, basically, one by one.
>>>
>>> I get that there might be some kind of a "nervousness" around this as we strive for not breaking it unnecessarily so there might be a lot of exceptions etc and I completely understand that but what I lack is clear visibility into what we plan to do with it (if anything).
>>>
>>> There is deprecated stuff as old as Cassandra 1.2 / 2.0 (!!!) and it is really questionable if we should not just get rid of that once for all. I am OK with keeping it there if we decide that, but we should provide some additional information like when it was deprecated and why it is necessary to keep it around otherwise the code-base will bloat and bloat ...
>>>
>>> (1) https://github.com/apache/cassandra/pull/2801/files
>>>
>>> ________________________________________
>>> From: Mick Semb Wever <mc...@apache.org>
>>> Sent: Friday, October 13, 2023 13:51
>>> 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.
>>>
>>>
>>>
>>>
>>>
>>> On Fri, 13 Oct 2023 at 13:07, Benjamin Lerer <bl...@apache.org>> wrote:
>>> 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.
>>>
>>>
>>> Yes, it is my understanding that such deprecation is only needed on methods/objects that belong to some API/SPI component of ours that requires compatibility. This is much more than configuration and JMX, and can be quite subtle in areas. A failed attempt I started at this is here: https://cwiki.apache.org/confluence/display/CASSANDRA/%28wip%29+Compatibility+Planning<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FCASSANDRA%2F%2528wip%2529%2BCompatibility%2BPlanning&data=05%7C01%7CStefan.Miklosovic%40netapp.com%7Cf0af5e7db5e9468faa8c08dbcbe2e9f3%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327947670748135%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3363U2ZlI%2FasNF0NTMrdZ%2BogB%2FjigmCGt3zqRrIs99Q%3D&reserved=0>
>>>
>>> But there will also be internal methods/objects marked as deprecated that relate back to these compatibility concerns, annotated because their connection and removal might not be so obvious when the time comes.
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by "Miklosovic, Stefan via dev" <de...@cassandra.apache.org>.
How I look at it is that if we clearly and explicitly specify "since" and "forRemoval" for all of our @Deprecated annotations, then it does really matter who is the consumer of that?
Imagine a scenario where there is some tool which puts cassandra-all on the class path and then it cherry-picks this and that from it. Once it is deprecated and it is explicitly visible since when and there is a clear policy what will happen with it in next release (some documetation on the website or similar), then is it really us to blame that their code will broke in the next release if they don't clean it up? I don't think so. External tooling should not take it for granted that what is there will be there for ever and what is deprecated will have its expiration date, again, if not explicitly said otherwise.
________________________________________
From: Josh McKenzie <jm...@apache.org>
Sent: Friday, October 13, 2023 15:36
To: dev
Cc: Miklosovic, Stefan
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.
If some piece of code is not used anymore then simplifying the code is the best thing to do
In the case of unused / unreferenced, sure. In the case of "other things use this but we shouldn't add any more dependencies on this because we need to remove it", a @Deprecated annotation w/version, reason, etc could be pretty useful.
Also - my instinct is that we have a lot of stuff in our ecosystem that depends on public methods in the codebase (I assume sidecar, bulk writer / reader, CDC clients though I tried to provide a formal API there, etc. etc) and I for one would be receptive to discussions on dev@ for the things people in the ecosystem have taken dependencies on so we can discuss whether or not to a) formally support those, and/or b) wrap an actual API around them so we can decouple those signatures from implementation.
Our lack of rigor around what's a public API and what's not combined with our historic default posture of "none of it's an API, if you depend on it it's on you and we'll break it, also we don't provide many public extension points nor do we provide more than the core functionality of the DB in our ecosystem so have fun" may not be the optimal posture for us in terms of ecosystem adoption + long-term maintenance burden. I realize we've done this in the name of us being able to be as productive as possible working on the core DB itself, but I'm not entirely convinced it's actually the most productive path tbh.
Go slow to go fast, invest to reap returns, etc.
On Fri, Oct 13, 2023, at 9:16 AM, Miklosovic, Stefan via dev wrote:
I forgot the round #3.
That would consist of an ant task which would scan the source. Since we enforced that each Deprecation annotation has to have its "since" on compile time, we can write a parser in that task which would tell you what you have to do in order to be sure that your next release will not contain any stuff which should not be there. E.g. when we release 6.0, all 4.0 stuff can go away etc ...
________________________________________
From: Miklosovic, Stefan via dev <de...@cassandra.apache.org>>
Sent: Friday, October 13, 2023 15:00
To: dev@cassandra.apache.org<ma...@cassandra.apache.org>
Cc: Miklosovic, Stefan
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.
OK. So here we are ... round 1 will be to map how bad it is, round 2 will be the removal of what should not be there. I am not sure if round 2 will be done before 5.0 is out (that would be ideal, to release 5.0 without a lot of baggage like that) so it will be better if we split this effort into two parts.
________________________________________
From: Benjamin Lerer <b....@gmail.com>>
Sent: Friday, October 13, 2023 14:45
To: dev@cassandra.apache.org<ma...@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.
Ok, thanks Stefan I understand the context better now. Looking at the PR.
Some make sense also for serialization reasons but some make no sense to me.
Le ven. 13 oct. 2023 à 14:26, Benjamin Lerer <b....@gmail.com>>> a écrit :
I’ve been told in the past not to remove public methods in a patch release though.
Then I am curious to get the rationale behind that. If some piece of code is not used anymore then simplifying the code is the best thing to do. It makes maintenance easier and avoids mistakes.
Le ven. 13 oct. 2023 à 14:11, Miklosovic, Stefan via dev <de...@cassandra.apache.org>>> a écrit :
Maybe for better understanding what we talk about, there is the PR which implements the changes suggested here (1)
It is clear that @Deprecated is not used exclusively on JMX / Configuration but we use it internally as well. This is a very delicate topic and we need to go, basically, one by one.
I get that there might be some kind of a "nervousness" around this as we strive for not breaking it unnecessarily so there might be a lot of exceptions etc and I completely understand that but what I lack is clear visibility into what we plan to do with it (if anything).
There is deprecated stuff as old as Cassandra 1.2 / 2.0 (!!!) and it is really questionable if we should not just get rid of that once for all. I am OK with keeping it there if we decide that, but we should provide some additional information like when it was deprecated and why it is necessary to keep it around otherwise the code-base will bloat and bloat ...
(1) https://github.com/apache/cassandra/pull/2801/files<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fcassandra%2Fpull%2F2801%2Ffiles&data=05%7C01%7CStefan.Miklosovic%40netapp.com%7C9693e079db614fd5200708dbcbf2311d%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638328013291548579%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=eDXmpk7AvWjHjbjS4KsmZbzkwT7RhNPVoJbnMlLZFQA%3D&reserved=0><https://github.com/apache/cassandra/pull/2801/files<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fcassandra%2Fpull%2F2801%2Ffiles&data=05%7C01%7CStefan.Miklosovic%40netapp.com%7C9693e079db614fd5200708dbcbf2311d%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638328013291548579%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=eDXmpk7AvWjHjbjS4KsmZbzkwT7RhNPVoJbnMlLZFQA%3D&reserved=0>>
________________________________________
From: Mick Semb Wever <mc...@apache.org>>>
Sent: Friday, October 13, 2023 13:51
To: dev@cassandra.apache.org<ma...@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.
On Fri, 13 Oct 2023 at 13:07, Benjamin Lerer <bl...@apache.org>>>> wrote:
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.
Yes, it is my understanding that such deprecation is only needed on methods/objects that belong to some API/SPI component of ours that requires compatibility. This is much more than configuration and JMX, and can be quite subtle in areas. A failed attempt I started at this is here: https://cwiki.apache.org/confluence/display/CASSANDRA/%28wip%29+Compatibility+Planning<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FCASSANDRA%2F%2528wip%2529%2BCompatibility%2BPlanning&data=05%7C01%7CStefan.Miklosovic%40netapp.com%7C9693e079db614fd5200708dbcbf2311d%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638328013291704815%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=FeHEgDdfOy7nBc0QSxyr01YhyniwtBILr02b08AWhnQ%3D&reserved=0><https://cwiki.apache.org/confluence/display/CASSANDRA/%28wip%29+Compatibility+Planning<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FCASSANDRA%2F%2528wip%2529%2BCompatibility%2BPlanning&data=05%7C01%7CStefan.Miklosovic%40netapp.com%7C9693e079db614fd5200708dbcbf2311d%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638328013291704815%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=FeHEgDdfOy7nBc0QSxyr01YhyniwtBILr02b08AWhnQ%3D&reserved=0>><https://cwiki.apache.org/confluence/display/CASSANDRA/%28wip%29+Compatibility+Planning<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FCASSANDRA%2F%2528wip%2529%2BCompatibility%2BPlanning&data=05%7C01%7CStefan.Miklosovic%40netapp.com%7C9693e079db614fd5200708dbcbf2311d%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638328013291704815%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=FeHEgDdfOy7nBc0QSxyr01YhyniwtBILr02b08AWhnQ%3D&reserved=0><https://cwiki.apache.org/confluence/display/CASSANDRA/%28wip%29+Compatibility+Planning<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FCASSANDRA%2F%2528wip%2529%2BCompatibility%2BPlanning&data=05%7C01%7CStefan.Miklosovic%40netapp.com%7C9693e079db614fd5200708dbcbf2311d%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638328013291704815%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=FeHEgDdfOy7nBc0QSxyr01YhyniwtBILr02b08AWhnQ%3D&reserved=0>>>
But there will also be internal methods/objects marked as deprecated that relate back to these compatibility concerns, annotated because their connection and removal might not be so obvious when the time comes.
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by Josh McKenzie <jm...@apache.org>.
> If some piece of code is not used anymore then simplifying the code is the best thing to do
In the case of unused / unreferenced, sure. In the case of "other things use this but we shouldn't add any more dependencies on this because we need to remove it", a @Deprecated annotation w/version, reason, etc could be pretty useful.
Also - my instinct is that we have a lot of stuff in our ecosystem that depends on public methods in the codebase (I assume sidecar, bulk writer / reader, CDC clients though I tried to provide a formal API there, etc. etc) and I for one would be receptive to discussions on dev@ for the things people in the ecosystem have taken dependencies on so we can discuss whether or not to a) formally support those, and/or b) wrap an actual API around them so we can decouple those signatures from implementation.
Our lack of rigor around what's a public API and what's not combined with our historic default posture of "none of it's an API, if you depend on it it's on you and we'll break it, also we don't provide many public extension points nor do we provide more than the core functionality of the DB in our ecosystem so have fun" *may not be* the optimal posture for us in terms of ecosystem adoption + long-term maintenance burden. I realize we've done this in the name of us being able to be as productive as possible working on the core DB itself, but I'm not entirely convinced it's actually the most productive path tbh.
Go slow to go fast, invest to reap returns, etc.
On Fri, Oct 13, 2023, at 9:16 AM, Miklosovic, Stefan via dev wrote:
> I forgot the round #3.
>
> That would consist of an ant task which would scan the source. Since we enforced that each Deprecation annotation has to have its "since" on compile time, we can write a parser in that task which would tell you what you have to do in order to be sure that your next release will not contain any stuff which should not be there. E.g. when we release 6.0, all 4.0 stuff can go away etc ...
>
> ________________________________________
> From: Miklosovic, Stefan via dev <de...@cassandra.apache.org>
> Sent: Friday, October 13, 2023 15:00
> To: dev@cassandra.apache.org
> Cc: Miklosovic, Stefan
> 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.
>
>
>
>
> OK. So here we are ... round 1 will be to map how bad it is, round 2 will be the removal of what should not be there. I am not sure if round 2 will be done before 5.0 is out (that would be ideal, to release 5.0 without a lot of baggage like that) so it will be better if we split this effort into two parts.
>
> ________________________________________
> From: Benjamin Lerer <b....@gmail.com>
> Sent: Friday, October 13, 2023 14:45
> 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.
>
>
>
> Ok, thanks Stefan I understand the context better now. Looking at the PR.
> Some make sense also for serialization reasons but some make no sense to me.
>
>
> Le ven. 13 oct. 2023 à 14:26, Benjamin Lerer <b....@gmail.com>> a écrit :
> I’ve been told in the past not to remove public methods in a patch release though.
>
> Then I am curious to get the rationale behind that. If some piece of code is not used anymore then simplifying the code is the best thing to do. It makes maintenance easier and avoids mistakes.
> Le ven. 13 oct. 2023 à 14:11, Miklosovic, Stefan via dev <de...@cassandra.apache.org>> a écrit :
> Maybe for better understanding what we talk about, there is the PR which implements the changes suggested here (1)
>
> It is clear that @Deprecated is not used exclusively on JMX / Configuration but we use it internally as well. This is a very delicate topic and we need to go, basically, one by one.
>
> I get that there might be some kind of a "nervousness" around this as we strive for not breaking it unnecessarily so there might be a lot of exceptions etc and I completely understand that but what I lack is clear visibility into what we plan to do with it (if anything).
>
> There is deprecated stuff as old as Cassandra 1.2 / 2.0 (!!!) and it is really questionable if we should not just get rid of that once for all. I am OK with keeping it there if we decide that, but we should provide some additional information like when it was deprecated and why it is necessary to keep it around otherwise the code-base will bloat and bloat ...
>
> (1) https://github.com/apache/cassandra/pull/2801/files<https://github.com/apache/cassandra/pull/2801/files>
>
> ________________________________________
> From: Mick Semb Wever <mc...@apache.org>>
> Sent: Friday, October 13, 2023 13:51
> To: dev@cassandra.apache.org<ma...@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.
>
>
>
>
>
> On Fri, 13 Oct 2023 at 13:07, Benjamin Lerer <bl...@apache.org>>> wrote:
> 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.
>
>
> Yes, it is my understanding that such deprecation is only needed on methods/objects that belong to some API/SPI component of ours that requires compatibility. This is much more than configuration and JMX, and can be quite subtle in areas. A failed attempt I started at this is here: https://cwiki.apache.org/confluence/display/CASSANDRA/%28wip%29+Compatibility+Planning<https://cwiki.apache.org/confluence/display/CASSANDRA/%28wip%29+Compatibility+Planning><https://cwiki.apache.org/confluence/display/CASSANDRA/%28wip%29+Compatibility+Planning<https://cwiki.apache.org/confluence/display/CASSANDRA/%28wip%29+Compatibility+Planning>>
>
> But there will also be internal methods/objects marked as deprecated that relate back to these compatibility concerns, annotated because their connection and removal might not be so obvious when the time comes.
>
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by "Miklosovic, Stefan via dev" <de...@cassandra.apache.org>.
I forgot the round #3.
That would consist of an ant task which would scan the source. Since we enforced that each Deprecation annotation has to have its "since" on compile time, we can write a parser in that task which would tell you what you have to do in order to be sure that your next release will not contain any stuff which should not be there. E.g. when we release 6.0, all 4.0 stuff can go away etc ...
________________________________________
From: Miklosovic, Stefan via dev <de...@cassandra.apache.org>
Sent: Friday, October 13, 2023 15:00
To: dev@cassandra.apache.org
Cc: Miklosovic, Stefan
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.
OK. So here we are ... round 1 will be to map how bad it is, round 2 will be the removal of what should not be there. I am not sure if round 2 will be done before 5.0 is out (that would be ideal, to release 5.0 without a lot of baggage like that) so it will be better if we split this effort into two parts.
________________________________________
From: Benjamin Lerer <b....@gmail.com>
Sent: Friday, October 13, 2023 14:45
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.
Ok, thanks Stefan I understand the context better now. Looking at the PR.
Some make sense also for serialization reasons but some make no sense to me.
Le ven. 13 oct. 2023 à 14:26, Benjamin Lerer <b....@gmail.com>> a écrit :
I’ve been told in the past not to remove public methods in a patch release though.
Then I am curious to get the rationale behind that. If some piece of code is not used anymore then simplifying the code is the best thing to do. It makes maintenance easier and avoids mistakes.
Le ven. 13 oct. 2023 à 14:11, Miklosovic, Stefan via dev <de...@cassandra.apache.org>> a écrit :
Maybe for better understanding what we talk about, there is the PR which implements the changes suggested here (1)
It is clear that @Deprecated is not used exclusively on JMX / Configuration but we use it internally as well. This is a very delicate topic and we need to go, basically, one by one.
I get that there might be some kind of a "nervousness" around this as we strive for not breaking it unnecessarily so there might be a lot of exceptions etc and I completely understand that but what I lack is clear visibility into what we plan to do with it (if anything).
There is deprecated stuff as old as Cassandra 1.2 / 2.0 (!!!) and it is really questionable if we should not just get rid of that once for all. I am OK with keeping it there if we decide that, but we should provide some additional information like when it was deprecated and why it is necessary to keep it around otherwise the code-base will bloat and bloat ...
(1) https://github.com/apache/cassandra/pull/2801/files<https://github.com/apache/cassandra/pull/2801/files>
________________________________________
From: Mick Semb Wever <mc...@apache.org>>
Sent: Friday, October 13, 2023 13:51
To: dev@cassandra.apache.org<ma...@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.
On Fri, 13 Oct 2023 at 13:07, Benjamin Lerer <bl...@apache.org>>> wrote:
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.
Yes, it is my understanding that such deprecation is only needed on methods/objects that belong to some API/SPI component of ours that requires compatibility. This is much more than configuration and JMX, and can be quite subtle in areas. A failed attempt I started at this is here: https://cwiki.apache.org/confluence/display/CASSANDRA/%28wip%29+Compatibility+Planning<https://cwiki.apache.org/confluence/display/CASSANDRA/%28wip%29+Compatibility+Planning><https://cwiki.apache.org/confluence/display/CASSANDRA/%28wip%29+Compatibility+Planning<https://cwiki.apache.org/confluence/display/CASSANDRA/%28wip%29+Compatibility+Planning>>
But there will also be internal methods/objects marked as deprecated that relate back to these compatibility concerns, annotated because their connection and removal might not be so obvious when the time comes.
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by "Miklosovic, Stefan via dev" <de...@cassandra.apache.org>.
OK. So here we are ... round 1 will be to map how bad it is, round 2 will be the removal of what should not be there. I am not sure if round 2 will be done before 5.0 is out (that would be ideal, to release 5.0 without a lot of baggage like that) so it will be better if we split this effort into two parts.
________________________________________
From: Benjamin Lerer <b....@gmail.com>
Sent: Friday, October 13, 2023 14:45
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.
Ok, thanks Stefan I understand the context better now. Looking at the PR.
Some make sense also for serialization reasons but some make no sense to me.
Le ven. 13 oct. 2023 à 14:26, Benjamin Lerer <b....@gmail.com>> a écrit :
I’ve been told in the past not to remove public methods in a patch release though.
Then I am curious to get the rationale behind that. If some piece of code is not used anymore then simplifying the code is the best thing to do. It makes maintenance easier and avoids mistakes.
Le ven. 13 oct. 2023 à 14:11, Miklosovic, Stefan via dev <de...@cassandra.apache.org>> a écrit :
Maybe for better understanding what we talk about, there is the PR which implements the changes suggested here (1)
It is clear that @Deprecated is not used exclusively on JMX / Configuration but we use it internally as well. This is a very delicate topic and we need to go, basically, one by one.
I get that there might be some kind of a "nervousness" around this as we strive for not breaking it unnecessarily so there might be a lot of exceptions etc and I completely understand that but what I lack is clear visibility into what we plan to do with it (if anything).
There is deprecated stuff as old as Cassandra 1.2 / 2.0 (!!!) and it is really questionable if we should not just get rid of that once for all. I am OK with keeping it there if we decide that, but we should provide some additional information like when it was deprecated and why it is necessary to keep it around otherwise the code-base will bloat and bloat ...
(1) https://github.com/apache/cassandra/pull/2801/files<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fcassandra%2Fpull%2F2801%2Ffiles&data=05%7C01%7CStefan.Miklosovic%40netapp.com%7C13df2c6740cf4bb2832e08dbcbea5924%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327979607217759%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=A8pSKjQJL894lvHB0RYn32U4t8cAIcA36XZ49njMmyI%3D&reserved=0>
________________________________________
From: Mick Semb Wever <mc...@apache.org>>
Sent: Friday, October 13, 2023 13:51
To: dev@cassandra.apache.org<ma...@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.
On Fri, 13 Oct 2023 at 13:07, Benjamin Lerer <bl...@apache.org>>> wrote:
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.
Yes, it is my understanding that such deprecation is only needed on methods/objects that belong to some API/SPI component of ours that requires compatibility. This is much more than configuration and JMX, and can be quite subtle in areas. A failed attempt I started at this is here: https://cwiki.apache.org/confluence/display/CASSANDRA/%28wip%29+Compatibility+Planning<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FCASSANDRA%2F%2528wip%2529%2BCompatibility%2BPlanning&data=05%7C01%7CStefan.Miklosovic%40netapp.com%7C13df2c6740cf4bb2832e08dbcbea5924%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327979607217759%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=gWRK8YI0F%2Bn%2BX402hGvQxKF%2FJPXyhzuQMS52IjoxQOw%3D&reserved=0><https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FCASSANDRA%2F%2528wip%2529%2BCompatibility%2BPlanning&data=05%7C01%7CStefan.Miklosovic%40netapp.com%7Cf0af5e7db5e9468faa8c08dbcbe2e9f3%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327947670748135%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3363U2ZlI%2FasNF0NTMrdZ%2BogB%2FjigmCGt3zqRrIs99Q%3D&reserved=0<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FCASSANDRA%2F%2528wip%2529%2BCompatibility%2BPlanning&data=05%7C01%7CStefan.Miklosovic%40netapp.com%7C13df2c6740cf4bb2832e08dbcbea5924%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327979607217759%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=gWRK8YI0F%2Bn%2BX402hGvQxKF%2FJPXyhzuQMS52IjoxQOw%3D&reserved=0>>
But there will also be internal methods/objects marked as deprecated that relate back to these compatibility concerns, annotated because their connection and removal might not be so obvious when the time comes.
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by Benjamin Lerer <b....@gmail.com>.
Ok, thanks Stefan I understand the context better now. Looking at the PR.
Some make sense also for serialization reasons but some make no sense to me.
Le ven. 13 oct. 2023 à 14:26, Benjamin Lerer <b....@gmail.com> a écrit :
> I’ve been told in the past not to remove public methods in a patch release
>> though.
>>
>
> Then I am curious to get the rationale behind that. If some piece of code
> is not used anymore then simplifying the code is the best thing to do. It
> makes maintenance easier and avoids mistakes.
> Le ven. 13 oct. 2023 à 14:11, Miklosovic, Stefan via dev <
> dev@cassandra.apache.org> a écrit :
>
>> Maybe for better understanding what we talk about, there is the PR which
>> implements the changes suggested here (1)
>>
>> It is clear that @Deprecated is not used exclusively on JMX /
>> Configuration but we use it internally as well. This is a very delicate
>> topic and we need to go, basically, one by one.
>>
>> I get that there might be some kind of a "nervousness" around this as we
>> strive for not breaking it unnecessarily so there might be a lot of
>> exceptions etc and I completely understand that but what I lack is clear
>> visibility into what we plan to do with it (if anything).
>>
>> There is deprecated stuff as old as Cassandra 1.2 / 2.0 (!!!) and it is
>> really questionable if we should not just get rid of that once for all. I
>> am OK with keeping it there if we decide that, but we should provide some
>> additional information like when it was deprecated and why it is necessary
>> to keep it around otherwise the code-base will bloat and bloat ...
>>
>> (1) https://github.com/apache/cassandra/pull/2801/files
>>
>> ________________________________________
>> From: Mick Semb Wever <mc...@apache.org>
>> Sent: Friday, October 13, 2023 13:51
>> 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.
>>
>>
>>
>>
>>
>> On Fri, 13 Oct 2023 at 13:07, Benjamin Lerer <blerer@apache.org<mailto:
>> blerer@apache.org>> wrote:
>> 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.
>>
>>
>> Yes, it is my understanding that such deprecation is only needed on
>> methods/objects that belong to some API/SPI component of ours that requires
>> compatibility. This is much more than configuration and JMX, and can be
>> quite subtle in areas. A failed attempt I started at this is here:
>> https://cwiki.apache.org/confluence/display/CASSANDRA/%28wip%29+Compatibility+Planning
>> <
>> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FCASSANDRA%2F%2528wip%2529%2BCompatibility%2BPlanning&data=05%7C01%7CStefan.Miklosovic%40netapp.com%7Cf0af5e7db5e9468faa8c08dbcbe2e9f3%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327947670748135%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3363U2ZlI%2FasNF0NTMrdZ%2BogB%2FjigmCGt3zqRrIs99Q%3D&reserved=0
>> >
>>
>> But there will also be internal methods/objects marked as deprecated that
>> relate back to these compatibility concerns, annotated because their
>> connection and removal might not be so obvious when the time comes.
>>
>
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by Benjamin Lerer <b....@gmail.com>.
>
> I’ve been told in the past not to remove public methods in a patch release
> though.
>
Then I am curious to get the rationale behind that. If some piece of code
is not used anymore then simplifying the code is the best thing to do. It
makes maintenance easier and avoids mistakes.
Le ven. 13 oct. 2023 à 14:11, Miklosovic, Stefan via dev <
dev@cassandra.apache.org> a écrit :
> Maybe for better understanding what we talk about, there is the PR which
> implements the changes suggested here (1)
>
> It is clear that @Deprecated is not used exclusively on JMX /
> Configuration but we use it internally as well. This is a very delicate
> topic and we need to go, basically, one by one.
>
> I get that there might be some kind of a "nervousness" around this as we
> strive for not breaking it unnecessarily so there might be a lot of
> exceptions etc and I completely understand that but what I lack is clear
> visibility into what we plan to do with it (if anything).
>
> There is deprecated stuff as old as Cassandra 1.2 / 2.0 (!!!) and it is
> really questionable if we should not just get rid of that once for all. I
> am OK with keeping it there if we decide that, but we should provide some
> additional information like when it was deprecated and why it is necessary
> to keep it around otherwise the code-base will bloat and bloat ...
>
> (1) https://github.com/apache/cassandra/pull/2801/files
>
> ________________________________________
> From: Mick Semb Wever <mc...@apache.org>
> Sent: Friday, October 13, 2023 13:51
> 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.
>
>
>
>
>
> On Fri, 13 Oct 2023 at 13:07, Benjamin Lerer <blerer@apache.org<mailto:
> blerer@apache.org>> wrote:
> 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.
>
>
> Yes, it is my understanding that such deprecation is only needed on
> methods/objects that belong to some API/SPI component of ours that requires
> compatibility. This is much more than configuration and JMX, and can be
> quite subtle in areas. A failed attempt I started at this is here:
> https://cwiki.apache.org/confluence/display/CASSANDRA/%28wip%29+Compatibility+Planning
> <
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FCASSANDRA%2F%2528wip%2529%2BCompatibility%2BPlanning&data=05%7C01%7CStefan.Miklosovic%40netapp.com%7Cf0af5e7db5e9468faa8c08dbcbe2e9f3%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327947670748135%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3363U2ZlI%2FasNF0NTMrdZ%2BogB%2FjigmCGt3zqRrIs99Q%3D&reserved=0
> >
>
> But there will also be internal methods/objects marked as deprecated that
> relate back to these compatibility concerns, annotated because their
> connection and removal might not be so obvious when the time comes.
>
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by "Miklosovic, Stefan via dev" <de...@cassandra.apache.org>.
Maybe for better understanding what we talk about, there is the PR which implements the changes suggested here (1)
It is clear that @Deprecated is not used exclusively on JMX / Configuration but we use it internally as well. This is a very delicate topic and we need to go, basically, one by one.
I get that there might be some kind of a "nervousness" around this as we strive for not breaking it unnecessarily so there might be a lot of exceptions etc and I completely understand that but what I lack is clear visibility into what we plan to do with it (if anything).
There is deprecated stuff as old as Cassandra 1.2 / 2.0 (!!!) and it is really questionable if we should not just get rid of that once for all. I am OK with keeping it there if we decide that, but we should provide some additional information like when it was deprecated and why it is necessary to keep it around otherwise the code-base will bloat and bloat ...
(1) https://github.com/apache/cassandra/pull/2801/files
________________________________________
From: Mick Semb Wever <mc...@apache.org>
Sent: Friday, October 13, 2023 13:51
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.
On Fri, 13 Oct 2023 at 13:07, Benjamin Lerer <bl...@apache.org>> wrote:
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.
Yes, it is my understanding that such deprecation is only needed on methods/objects that belong to some API/SPI component of ours that requires compatibility. This is much more than configuration and JMX, and can be quite subtle in areas. A failed attempt I started at this is here: https://cwiki.apache.org/confluence/display/CASSANDRA/%28wip%29+Compatibility+Planning<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FCASSANDRA%2F%2528wip%2529%2BCompatibility%2BPlanning&data=05%7C01%7CStefan.Miklosovic%40netapp.com%7Cf0af5e7db5e9468faa8c08dbcbe2e9f3%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327947670748135%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3363U2ZlI%2FasNF0NTMrdZ%2BogB%2FjigmCGt3zqRrIs99Q%3D&reserved=0>
But there will also be internal methods/objects marked as deprecated that relate back to these compatibility concerns, annotated because their connection and removal might not be so obvious when the time comes.
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by Mick Semb Wever <mc...@apache.org>.
On Fri, 13 Oct 2023 at 13:07, Benjamin Lerer <bl...@apache.org> wrote:
> 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.
>
Yes, it is my understanding that such deprecation is only needed on
methods/objects that belong to some API/SPI component of ours that requires
compatibility. This is much more than configuration and JMX, and can be
quite subtle in areas. A failed attempt I started at this is here:
https://cwiki.apache.org/confluence/display/CASSANDRA/%28wip%29+Compatibility+Planning
But there will also be internal methods/objects marked as deprecated that
relate back to these compatibility concerns, annotated because their
connection and removal might not be so obvious when the time comes.
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by Ekaterina Dimitrova <e....@gmail.com>.
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 <bl...@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 <bl...@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<ma...@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....@gmail.com>>
>> Sent: Friday, October 13, 2023 12:19
>> To: dev@cassandra.apache.org<ma...@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 <frankgh@apache.org
>> <ma...@apache.org><mailto:frankgh@apache.org<mailto:
>> frankgh@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.Miklosovic@netapp.com<mailto:Stefan.Miklosovic@netapp.com
>> ><ma...@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 <frankgh@apache.org<mailto:
>> frankgh@apache.org><mailto:frankgh@apache.org<mailto:frankgh@apache.org
>> >>>
>> > > Sent: Tuesday, October 10, 2023 23:34
>> > > To: dev@cassandra.apache.org<ma...@cassandra.apache.org><mailto:
>> dev@cassandra.apache.org<ma...@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 <jmckenzie@apache.org
>> <ma...@apache.org><mailto:jmckenzie@apache.org<mailto:
>> jmckenzie@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 <mck@apache.org<mailto:mck@apache.org
>> ><ma...@apache.org>>>
>> > > > > Sent: Monday, October 9, 2023 10:55
>> > > > > To: dev@cassandra.apache.org<mailto:dev@cassandra.apache.org
>> ><ma...@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.
>> > > > >
>> > > > >
>> > > >
>> >
>>
>
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by Benjamin Lerer <bl...@apache.org>.
>
> 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 <bl...@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<ma...@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....@gmail.com>>
> Sent: Friday, October 13, 2023 12:19
> To: dev@cassandra.apache.org<ma...@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 <frankgh@apache.org
> <ma...@apache.org><mailto:frankgh@apache.org<mailto:
> frankgh@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.Miklosovic@netapp.com<mailto:Stefan.Miklosovic@netapp.com
> ><ma...@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 <frankgh@apache.org<mailto:frankgh@apache.org
> ><ma...@apache.org>>>
> > > Sent: Tuesday, October 10, 2023 23:34
> > > To: dev@cassandra.apache.org<ma...@cassandra.apache.org><mailto:
> dev@cassandra.apache.org<ma...@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 <jmckenzie@apache.org
> <ma...@apache.org><mailto:jmckenzie@apache.org<mailto:
> jmckenzie@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 <mck@apache.org<mailto:mck@apache.org
> ><ma...@apache.org>>>
> > > > > Sent: Monday, October 9, 2023 10:55
> > > > > To: dev@cassandra.apache.org<mailto:dev@cassandra.apache.org
> ><ma...@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.
> > > > >
> > > > >
> > > >
> >
>
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by "Miklosovic, Stefan via dev" <de...@cassandra.apache.org>.
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 <bl...@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 <de...@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....@gmail.com>>
Sent: Friday, October 13, 2023 12:19
To: dev@cassandra.apache.org<ma...@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 <fr...@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
> <St...@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 <fr...@apache.org>>>
> > Sent: Tuesday, October 10, 2023 23:34
> > To: dev@cassandra.apache.org<ma...@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 <jm...@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 <mc...@apache.org>>>
> > > > Sent: Monday, October 9, 2023 10:55
> > > > To: dev@cassandra.apache.org<ma...@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.
> > > >
> > > >
> > >
>
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by Benjamin Lerer <bl...@apache.org>.
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....@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 <frankgh@apache.org
> <ma...@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
> > <St...@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 <frankgh@apache.org<mailto:frankgh@apache.org
> >>
> > > Sent: Tuesday, October 10, 2023 23:34
> > > To: dev@cassandra.apache.org<ma...@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 <jmckenzie@apache.org
> <ma...@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 <mc...@apache.org>>
> > > > > Sent: Monday, October 9, 2023 10:55
> > > > > To: dev@cassandra.apache.org<ma...@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.
> > > > >
> > > > >
> > > >
> >
>
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by "Miklosovic, Stefan via dev" <de...@cassandra.apache.org>.
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....@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 <fr...@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
> <St...@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 <fr...@apache.org>>
> > Sent: Tuesday, October 10, 2023 23:34
> > To: dev@cassandra.apache.org<ma...@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 <jm...@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 <mc...@apache.org>>
> > > > Sent: Monday, October 9, 2023 10:55
> > > > To: dev@cassandra.apache.org<ma...@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.
> > > >
> > > >
> > >
>
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by Benjamin Lerer <b....@gmail.com>.
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 <fr...@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
> > <St...@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 <fr...@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 <jm...@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 <mc...@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.
> > > > >
> > > > >
> > > >
> >
>
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by Francisco Guerrero <fr...@apache.org>.
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
> <St...@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 <fr...@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 <jm...@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 <mc...@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.
> > > >
> > > >
> > >
>
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by "Miklosovic, Stefan" <St...@netapp.com>.
I hope I am not nitpicking here but there is also "@see" annotation which could contain that JIRA ticket.
/**
* @see <a href="https://issues.apache.org/jira/browse/CASSANDRA-123">CASSANDRA-123</a>
*/
Doing ctrl+q (at least that is how I have it in IDEA) will show Javadoc for such javadoc'ed element and you can just click to that directly and it will open a tab for you in a browser. I am not sure there is a faster way to get to that.
________________________________________
From: Maxim Muzafarov <mm...@apache.org>
Sent: Wednesday, October 11, 2023 18:59
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.
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
<St...@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 <fr...@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 <jm...@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 <mc...@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.
> > >
> > >
> >
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by Maxim Muzafarov <mm...@apache.org>.
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
<St...@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 <fr...@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 <jm...@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 <mc...@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.
> > >
> > >
> >
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by "Miklosovic, Stefan" <St...@netapp.com>.
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 <fr...@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 <jm...@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 <mc...@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.
> >
> >
>
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by Francisco Guerrero <fr...@apache.org>.
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 <jm...@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 <mc...@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.
> >
> >
>
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by Maxim Muzafarov <mm...@apache.org>.
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 <jm...@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 <mc...@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.
>
>
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by Josh McKenzie <jm...@apache.org>.
Sounds like we're relitigating the basics of how @Deprecated, forRemoval, since, and javadoc @link all intersect to make deprecation less painful ;)
So:
1. Built-in java.lang.Deprecated: required
2. 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)
3. 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 <mc...@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.
>
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by "Miklosovic, Stefan" <St...@netapp.com>.
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 <mc...@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.
Re: [DISCUSS] putting versions into Deprecated annotations
Posted by Mick Semb Wever <mc...@apache.org>.
>
> 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.