Re: [DISCUSS] putting versions into Deprecated annotations
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 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 mailto:dev@cassandra.apache.org>> Sent: Friday, October 13, 2023 15:00 To: dev@cassandra.apache.org<mailto: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 mailto:b.le...@gmail.com>> Sent: Friday, October 13, 2023 14:45 To: dev@cassandra.apache.org<mailto:dev@cassandra.apache.org> Subject: Re: [DISCUSS] putting versions into Deprecated annotations NetApp Security WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe. 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 mailto:b.le...@gmail.com><mailto:b.le...@gmail.com<mailto:b.le...@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 easi
Re: [DISCUSS] putting versions into Deprecated annotations
> 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 > 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 > 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 > mailto:b.le...@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 > mailto: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
Re: [DISCUSS] putting versions into Deprecated annotations
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 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 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 mailto:b.le...@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 mailto: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<https://github.com/apache/cassandra/pull/2801/files> From: Mick Semb Wever mailto:m...@apache.org>> Sent: Friday, October 13, 2023 13:51 To: dev@cassandra.apache.org<mailto:dev@cassandra.apache.org> Subject: Re: [DISCUSS] putting versions into Deprecated annotations NetApp Security WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe. On Fri, 13 Oct 2023 at 13:07, Benjamin Lerer mailto:ble...@apache.org><mailto:ble...@apache.org<mailto:ble...@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
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 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 mailto:b.le...@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 mailto: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<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fcassandra%2Fpull%2F2801%2Ffiles=05%7C01%7CStefan.Miklosovic%40netapp.com%7C13df2c6740cf4bb2832e08dbcbea5924%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327979607217759%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=A8pSKjQJL894lvHB0RYn32U4t8cAIcA36XZ49njMmyI%3D=0> From: Mick Semb Wever mailto:m...@apache.org>> Sent: Friday, October 13, 2023 13:51 To: dev@cassandra.apache.org<mailto:dev@cassandra.apache.org> Subject: Re: [DISCUSS] putting versions into Deprecated annotations NetApp Security WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe. On Fri, 13 Oct 2023 at 13:07, Benjamin Lerer mailto:ble...@apache.org><mailto:ble...@apache.org<mailto:ble...@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=05%7C01%7CStefan.Miklosovic%40netapp.com%7C13df2c6740cf4bb2832e08dbcbea5924%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327979607217759%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=gWRK8YI0F%2Bn%2BX402hGvQxKF%2FJPXyhzuQMS52IjoxQOw%3D=0><https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FCASSANDRA%2F%2528wip%2529%2BCompatibility%2BPlanning=05%7C01%7CStefan.Miklosovic%40netapp.com%7Cf0af5e7db5e9468faa8c08dbcbe2e9f3%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327947670748135%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=3363U2ZlI%2FasNF0NTMrdZ%2BogB%2FjigmCGt3zqRrIs99Q%3D=0<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FCASSANDRA%2F%2528wip%2529%2BCompatibility%2BPlanning=05%7C01%7CStefan.Miklosovic%40netapp.com%7C1
Re: [DISCUSS] putting versions into Deprecated annotations
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 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 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 >> 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 >>> 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 >>> mailto:ble...@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=05%7C01%7CStefan.Miklosovic%40netapp.com%7Cf0af5e7db5e9468faa8c08dbcbe2e9f3%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327947670748135%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=3363U2ZlI%2FasNF0NTMrdZ%2BogB%2FjigmCGt3zqRrIs99Q%3D=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
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 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 >> 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 > ble...@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=05%7C01%7CStefan.Miklosovic%40netapp.com%7Cf0af5e7db5e9468faa8c08dbcbe2e9f3%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327947670748135%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=3363U2ZlI%2FasNF0NTMrdZ%2BogB%2FjigmCGt3zqRrIs99Q%3D=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
> > 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 > 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 ble...@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=05%7C01%7CStefan.Miklosovic%40netapp.com%7Cf0af5e7db5e9468faa8c08dbcbe2e9f3%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327947670748135%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=3363U2ZlI%2FasNF0NTMrdZ%2BogB%2FjigmCGt3zqRrIs99Q%3D=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
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 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 mailto:ble...@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=05%7C01%7CStefan.Miklosovic%40netapp.com%7Cf0af5e7db5e9468faa8c08dbcbe2e9f3%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327947670748135%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=3363U2ZlI%2FasNF0NTMrdZ%2BogB%2FjigmCGt3zqRrIs99Q%3D=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
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 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 >> Sent: Friday, October 13, 2023 13:07 >> To: dev@cassandra.apache.org >> Subject: Re: [DISCUSS] putting versions into Deprecated annotations >> >> NetApp Security WARNING: This is an external email. Do not click links or >> open attachments unless you recognize the sender and know the content is >> safe. >> >> >> >> I was asking because outside of configuration parameters and JMX calls, >> the approach as far as I remember was to just change things without using >> an annotation. >> >> Le ven. 13 oct. 2023 à 12:45, Miklosovic, Stefan via dev < >> dev@cassandra.apache.org<mailto:dev@cassandra.apache.org>> a écrit : >> Hi Benjamin, >> >> in other words, anything we have @Deprecated annotation on top of (or >> anything you want to annotate with it). Does it help with the explanation? >> >> For the initial phase, I plan to just put "since" everywhere (into every >> already existing @Deprecated annotation) and we leave out "forRemoval" in >> Deprecated annotation for now as that is quite tricky to get right. >> >> I am confused what is considered to be removed and what we keep there for >> ever even it is deprecated (referring to what Mick said in this thread that >> forRemoval can not be by default true). After we map what technical debt we >> have, we can summarize this and I bring it to the ML again for further >> discussion what to actually remove and when. >> >> Regards >> >> >> From: Benjamin Lerer mailto:b.le...@gmail.com>> >> Sent: Friday, October 13, 2023 12:19 >> To: dev@cassandra.apache.org<mailto:dev@cassandra.apache.org> >> Subject: Re: [DISCUSS] putting versions into Deprecated annotations >> >> NetApp Security WARNING: This is an external email. Do not click links or >> open attachments unless you recognize the sender and know the content is >> safe. >> >> >> >> I am a bit confused by the starting point of this discussion: "When we >> deprecate APIs / methods" >> What are we exactly calling APIs/methods? It is really unclear to me what >> we are talking about here. >> >> Le jeu. 12 oct. 2023 à 02:38, Francisco Guerrero > <mailto:fran...@apache.org><mailto:fran...@apache.org> fran...@apache.org>>> a écrit : >> >> >> On 2023/10/11 16:59:35 Maxim Muzafarov wrote: >> > Francisco, >> > >> > I agree with your vision of the deprecation comments and actually, I >> > think we should recommend doing it that way for the cases where it is >> > applicable on our code-style page, but when things get to the >> > implementation phase there are some obstacles that are not easy to >> > overcome. >> >> Yeah, I agree that this should be recommended rather than enforced via >> some checkstyle rule. However, reviewers should be aware of this >> recommendation in the code-style page. >> >> > >> > So, adding the MissingDeprecated will emphasize to a developer the >> > need to describe the deprecation reasons in comments, but >> > u
Re: [DISCUSS] putting versions into Deprecated annotations
> > 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 > Sent: Friday, October 13, 2023 13:07 > To: dev@cassandra.apache.org > Subject: Re: [DISCUSS] putting versions into Deprecated annotations > > NetApp Security WARNING: This is an external email. Do not click links or > open attachments unless you recognize the sender and know the content is > safe. > > > > I was asking because outside of configuration parameters and JMX calls, > the approach as far as I remember was to just change things without using > an annotation. > > Le ven. 13 oct. 2023 à 12:45, Miklosovic, Stefan via dev < > dev@cassandra.apache.org<mailto:dev@cassandra.apache.org>> a écrit : > Hi Benjamin, > > in other words, anything we have @Deprecated annotation on top of (or > anything you want to annotate with it). Does it help with the explanation? > > For the initial phase, I plan to just put "since" everywhere (into every > already existing @Deprecated annotation) and we leave out "forRemoval" in > Deprecated annotation for now as that is quite tricky to get right. > > I am confused what is considered to be removed and what we keep there for > ever even it is deprecated (referring to what Mick said in this thread that > forRemoval can not be by default true). After we map what technical debt we > have, we can summarize this and I bring it to the ML again for further > discussion what to actually remove and when. > > Regards > > ________________ > From: Benjamin Lerer mailto:b.le...@gmail.com>> > Sent: Friday, October 13, 2023 12:19 > To: dev@cassandra.apache.org<mailto:dev@cassandra.apache.org> > Subject: Re: [DISCUSS] putting versions into Deprecated annotations > > NetApp Security WARNING: This is an external email. Do not click links or > open attachments unless you recognize the sender and know the content is > safe. > > > > I am a bit confused by the starting point of this discussion: "When we > deprecate APIs / methods" > What are we exactly calling APIs/methods? It is really unclear to me what > we are talking about here. > > Le jeu. 12 oct. 2023 à 02:38, Francisco Guerrero <mailto:fran...@apache.org><mailto:fran...@apache.org fran...@apache.org>>> a écrit : > > > On 2023/10/11 16:59:35 Maxim Muzafarov wrote: > > Francisco, > > > > I agree with your vision of the deprecation comments and actually, I > > think we should recommend doing it that way for the cases where it is > > applicable on our code-style page, but when things get to the > > implementation phase there are some obstacles that are not easy to > > overcome. > > Yeah, I agree that this should be recommended rather than enforced via > some checkstyle rule. However, reviewers should be aware of this > recommendation in the code-style page. > > > > > So, adding the MissingDeprecated will emphasize to a developer the > > need to describe the deprecation reasons in comments, but > > unfortunately, there is no general pattern that we can enforce for > > every such description message and/or automatically validate its > > meaningfulness. There may be no alternative for a deprecated field, or > > it may simply be marked for deletion, so the pattern is slightly > > different in this case. > > > +1 for adding the MissingDeprecated rule > > > Another problem is how to add meaningful comments to the deprecated > > annotations that we already have in the code, sin
Re: [DISCUSS] putting versions into Deprecated annotations
On Fri, 13 Oct 2023 at 13:07, Benjamin Lerer 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
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 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 mailto:dev@cassandra.apache.org>> a écrit : Hi Benjamin, in other words, anything we have @Deprecated annotation on top of (or anything you want to annotate with it). Does it help with the explanation? For the initial phase, I plan to just put "since" everywhere (into every already existing @Deprecated annotation) and we leave out "forRemoval" in Deprecated annotation for now as that is quite tricky to get right. I am confused what is considered to be removed and what we keep there for ever even it is deprecated (referring to what Mick said in this thread that forRemoval can not be by default true). After we map what technical debt we have, we can summarize this and I bring it to the ML again for further discussion what to actually remove and when. Regards From: Benjamin Lerer mailto:b.le...@gmail.com>> Sent: Friday, October 13, 2023 12:19 To: dev@cassandra.apache.org<mailto:dev@cassandra.apache.org> Subject: Re: [DISCUSS] putting versions into Deprecated annotations NetApp Security WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe. I am a bit confused by the starting point of this discussion: "When we deprecate APIs / methods" What are we exactly calling APIs/methods? It is really unclear to me what we are talking about here. Le jeu. 12 oct. 2023 à 02:38, Francisco Guerrero mailto:fran...@apache.org><mailto:fran...@apache.org<mailto:fran...@apache.org>>> a écrit : On 2023/10/11 16:59:35 Maxim Muzafarov wrote: > Francisco, > > I agree with your vision of the deprecation comments and actually, I > think we should recommend doing it that way for the cases where it is > applicable on our code-style page, but when things get to the > implementation phase there are some obstacles that are not easy to > overcome. Yeah, I agree that this should be recommended rather than enforced via some checkstyle rule. However, reviewers should be aware of this recommendation in the code-style page. > > So, adding the MissingDeprecated will emphasize to a developer the > need to describe the deprecation reasons in comments, but > unfortunately, there is no general pattern that we can enforce for > every such description message and/or automatically validate its > meaningfulness. There may be no alternative for a deprecated field, or > it may simply be marked for deletion, so the pattern is slightly > different in this case. +1 for adding the MissingDeprecated rule > Another problem is how to add meaningful comments to the deprecated > annotations that we already have in the code, since we can't enforce > checkstyle rules only on newly added code. This is a very exhausting > process with no 100% guarantee of accuracy - some of the commits don't > have a good commit message and require a deep archaeology. Not aiming for 100% accuracy, but more on code style agreement. > All of the above led me to the following which is pretty easy to > achieve and improves the code quality: > > /** @deprecated See CASSANDRA-6504 */ > @Deprecated(since = "2.1") > public Integer concurrent_replicates = null; > > On Wed, 11 Oct 2023 at 09:51, Miklosovic, Stefan > mailto:stefan.mikloso...@netapp.com><mailto:stefan.mikloso...@netapp.com<mailto:stefan.mikloso...@netapp.com>>> > wrote: > > > > Here (1) it supports check of both Javadoc and annotation at the same time > > so what you want is possible. What is not possible is to checkstyle the > > _content_ of deprecated Javadoc nor any format of it. I think that ensuring > > the presence of both annotation and Javadoc comment is just enough. > > > > (1) > > https://checkstyle.sourceforge.io/apidocs/com/puppycrawl/tools/checkstyle/checks/annotation/MissingDeprecatedCheck.html<https://nam04.safelinks.protection
Re: [DISCUSS] putting versions into Deprecated annotations
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 > 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 <mailto:fran...@apache.org>> a écrit : > > > On 2023/10/11 16:59:35 Maxim Muzafarov wrote: > > Francisco, > > > > I agree with your vision of the deprecation comments and actually, I > > think we should recommend doing it that way for the cases where it is > > applicable on our code-style page, but when things get to the > > implementation phase there are some obstacles that are not easy to > > overcome. > > Yeah, I agree that this should be recommended rather than enforced via > some checkstyle rule. However, reviewers should be aware of this > recommendation in the code-style page. > > > > > So, adding the MissingDeprecated will emphasize to a developer the > > need to describe the deprecation reasons in comments, but > > unfortunately, there is no general pattern that we can enforce for > > every such description message and/or automatically validate its > > meaningfulness. There may be no alternative for a deprecated field, or > > it may simply be marked for deletion, so the pattern is slightly > > different in this case. > > > +1 for adding the MissingDeprecated rule > > > Another problem is how to add meaningful comments to the deprecated > > annotations that we already have in the code, since we can't enforce > > checkstyle rules only on newly added code. This is a very exhausting > > process with no 100% guarantee of accuracy - some of the commits don't > > have a good commit message and require a deep archaeology. > > Not aiming for 100% accuracy, but more on code style agreement. > > > All of the above led me to the following which is pretty easy to > > achieve and improves the code quality: > > > > /** @deprecated See CASSANDRA-6504 */ > > @Deprecated(since = "2.1") > > public Integer concurrent_replicates = null; > > > > On Wed, 11 Oct 2023 at 09:51, Miklosovic, Stefan > > mailto:stefan.mikloso...@netapp.com>> > wrote: > > > > > > Here (1) it supports check of both Javadoc and annotation at the same > time so what you want is possible. What is not possible is to checkstyle > the _content_ of deprecated Javadoc nor any format of it. I think that > ensuring the presence of both annotation and Javadoc comment is just enough. > > > > > > (1) > https://checkstyle.sourceforge.io/apidocs/com/puppycrawl/tools/checkstyle/checks/annotation/MissingDeprecatedCheck.html > < > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcheckstyle.sourceforge.io%2Fapidocs%2Fcom%2Fpuppycrawl%2Ftools%2Fcheckstyle%2Fchecks%2Fannotation%2FMissingDeprecatedCheck.html=05%7C01%7CStefan.Miklosovic%40netapp.com%7C59fa2b3786ff436c83ba08dbcbd5ece7%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327891917050879%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=8qKu8ob%2BvPdHfUQdkxr5C%2BgkR5iMcUaEqw9a%2FNN276k%3D=0 > > > > > > > > > &g
Re: [DISCUSS] putting versions into Deprecated annotations
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 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 mailto:fran...@apache.org>> a écrit : On 2023/10/11 16:59:35 Maxim Muzafarov wrote: > Francisco, > > I agree with your vision of the deprecation comments and actually, I > think we should recommend doing it that way for the cases where it is > applicable on our code-style page, but when things get to the > implementation phase there are some obstacles that are not easy to > overcome. Yeah, I agree that this should be recommended rather than enforced via some checkstyle rule. However, reviewers should be aware of this recommendation in the code-style page. > > So, adding the MissingDeprecated will emphasize to a developer the > need to describe the deprecation reasons in comments, but > unfortunately, there is no general pattern that we can enforce for > every such description message and/or automatically validate its > meaningfulness. There may be no alternative for a deprecated field, or > it may simply be marked for deletion, so the pattern is slightly > different in this case. +1 for adding the MissingDeprecated rule > Another problem is how to add meaningful comments to the deprecated > annotations that we already have in the code, since we can't enforce > checkstyle rules only on newly added code. This is a very exhausting > process with no 100% guarantee of accuracy - some of the commits don't > have a good commit message and require a deep archaeology. Not aiming for 100% accuracy, but more on code style agreement. > All of the above led me to the following which is pretty easy to > achieve and improves the code quality: > > /** @deprecated See CASSANDRA-6504 */ > @Deprecated(since = "2.1") > public Integer concurrent_replicates = null; > > On Wed, 11 Oct 2023 at 09:51, Miklosovic, Stefan > mailto:stefan.mikloso...@netapp.com>> wrote: > > > > Here (1) it supports check of both Javadoc and annotation at the same time > > so what you want is possible. What is not possible is to checkstyle the > > _content_ of deprecated Javadoc nor any format of it. I think that ensuring > > the presence of both annotation and Javadoc comment is just enough. > > > > (1) > > https://checkstyle.sourceforge.io/apidocs/com/puppycrawl/tools/checkstyle/checks/annotation/MissingDeprecatedCheck.html<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcheckstyle.sourceforge.io%2Fapidocs%2Fcom%2Fpuppycrawl%2Ftools%2Fcheckstyle%2Fchecks%2Fannotation%2FMissingDeprecatedCheck.html=05%7C01%7CStefan.Miklosovic%40netapp.com%7C59fa2b3786ff436c83ba08dbcbd5ece7%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C638327891917050879%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=8qKu8ob%2BvPdHfUQdkxr5C%2BgkR5iMcUaEqw9a%2FNN276k%3D=0> > > > > ________________ > > From: Francisco Guerrero mailto:fran...@apache.org>> > > Sent: Tuesday, October 10, 2023 23:34 > > To: dev@cassandra.apache.org<mailto:dev@cassandra.apache.org> > > Subject: Re: [DISCUSS] putting versions into Deprecated annotations > > > > NetApp Security WARNING: This is an external email. Do not click links or > > open attachments unless you recognize the sender and know the content is > > safe. > > > > > > > > > > To me this seems insufficient. As a developer, I'd like to see what the > > alternative is when reading the javadoc without having to go to Jira
Re: [DISCUSS] putting versions into Deprecated annotations
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 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
> > 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
> > > 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
> > > > visib
Re: [DISCUSS] putting versions into Deprecated annotations
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
> 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
> > 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 wrote:
> > > >
> &g
Re: [DISCUSS] putting versions into Deprecated annotations
I hope I am not nitpicking here but there is also "@see" annotation which could
contain that JIRA ticket.
/**
* @see https://issues.apache.org/jira/browse/CASSANDRA-123;>CASSANDRA-123
*/
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
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
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
> 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 wrote:
> > >
> > >
Re: [DISCUSS] putting versions into Deprecated annotations
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
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
> 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 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
Re: [DISCUSS] putting versions into Deprecated annotations
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
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 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
> > 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
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 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
> > 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
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 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
> 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
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
> 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
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 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
> > 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
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
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-N", 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
> 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-N", 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
+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 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-N", 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
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-N", 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
