pegasas commented on PR #17190:
URL: https://github.com/apache/kafka/pull/17190#issuecomment-2520077595
> I believe the annotation and comment is wrong. While it does test the old
API, there is no corresponding test for the new API (or did I miss it?), and
thus, it should have be `@SuppressWarning("deprecation")` (instead of
`@Deprecated`) and the comment should say `// Old PAPI. Needs to be migrated.`
similar to the other cases for which I did leave a comment.
>
> We should not remove these 4 tests, but rather rewrite all 4 to use the
new PAPI, to avoid reducing test coverage. For the other test you remove in
this PR, eg,
`testDrivingConnectedStateStoreInDifferentProcessorsTopologyWithOldAPI`, there
is already equivalent tests for the new PAPI
(`testDrivingConnectedStateStoreInDifferentProcessorsTopology`) so it's ok to
remove the "duplicate" test using the old API.
>
> Does this make sense?
although the method you mentioned is SuppressWarning, but actually related
class has been deprecated. maybe I think we should remove this as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
