AuroraTwinkle commented on PR #24293: URL: https://github.com/apache/pulsar/pull/24293#issuecomment-2876180899
> > Why don't other methods add the Async suffix but just `checkTopicExists`? For example: `checkNonPartitionedTopicExists` > > @AuroraTwinkle While reviewing #24225, I noticed an issue where the newly added method `checkTopicExistsAsync` simply calls the synchronous `checkTopicExists`. This pattern is confusing, and since the PR author did not intend to address it, I resolved the problem myself. > > If you'd like, could you check whether there are other instances in Pulsar where **asynchronous methods improperly reuse synchronous method names** (or vice versa)** and fix them? If you don't have time, I can take some time next week to do a deeper inspection of the Pulsar codebase and fix similar patterns In fact, what I want to say is that I don't think it's necessary to add the Async suffix to existing methods, especially those that are not client-side methods. This will result in many existing functions having two signatures (one with the Async suffix and the other without), which is more likely to cause misunderstandings and destroy the beauty of the code. Perhaps what should be discussed is: 1. Is it necessary to stipulate that the name of the asynchronous method should be suffixed with Async (in my opinion, there is no need to force the Async suffix) 2. If necessary, then a related check style should be established to prevent non-standard code submission 3. Existing methods should remain unchanged: If you just add a new method signature with the Async suffix, I think it doesn’t make much sense and will greatly destroy the beauty of the code (Because there are many existing asynchronous methods without the Async suffix). -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
