-
Notifications
You must be signed in to change notification settings - Fork 637
[ISSUE #4211] Add JavaDoc for eventmesh.runtime.admin APIs (not in dashboard) #4272
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Pil0tXia IMO,Both methods and classes can be accompanied by explanatory comments, but class properties and method code do not require excessive comments. Comments serve the purpose of enhancing code comprehension. Regarding class properties, if the property names are semantically clear and self-explanatory, there is no need to add comment explanations. For complex logic, adding appropriate comments can be helpful, while for relatively simple logic, comments may not be necessary.
One more small suggestion, try to compress multiple commits into one for a Pull Request (PR). Keeping the commit messages concise and focused helps other developers better understand the code evolution.
Codecov Report
@@ Coverage Diff @@
## master #4272 +/- ##
============================================
- Coverage 16.90% 16.86% -0.05%
- Complexity 1416 1426 +10
============================================
Files 589 593 +4
Lines 25837 26068 +231
Branches 2394 2401 +7
============================================
+ Hits 4369 4397 +28
- Misses 21031 21230 +199
- Partials 437 441 +4
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@mxsm Thank you for your valuable opinion. For the existing single-line comments in the code, I hope they can be presented in natural language to reduce the time others spend on inspecting the source code. This way, one can more directly understand a relatively complex data structure and its purpose for being used here. For instance, in this picture, the specific meaning of traversing keys and values in nested iterations requires looking at the property definitions of the entity class to understand. However, single-line comments can precisely indicate different levels and directly inform readers about the relationship between these entity classes and the concept of "client group." By doing so, readers only need to go through the comments to understand the workflow of this handler and to clearly divide sections for retrieval, validation, processing, and response logic. Of course, I will also follow your suggestion to further streamline single-line comments with relatively clear meanings. As for what you mentioned about "class properties", if you are referring to the attributes of entity classes, rest assured that I have hardly added comments for class properties, except for improving the existing ones to better express their purposes. Lastly, I will squash multiple commits into a new branch for the pull request. Thank you for your excellent suggestions. |
@mxsm I have streamlined and reduced existing single-line comments. 😉 In terms of squash commits, creating a new branch, squashing, and submitting it will result in the old branch not being able to synchronize modifications during the review process, and the new branch will also be unable to see the old commit history. This is not ideal. I think that splitting pull requests would be a better approach, ensuring that the number of commits in each pull request is fewer than 15. |
...ime/src/main/java/org/apache/eventmesh/runtime/admin/handler/DeleteWebHookConfigHandler.java
Outdated
Show resolved
Hide resolved
...ime/src/main/java/org/apache/eventmesh/runtime/admin/handler/DeleteWebHookConfigHandler.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/apache/eventmesh/runtime/admin/handler/QueryRecommendEventMeshHandler.java
Outdated
Show resolved
Hide resolved
...e/src/main/java/org/apache/eventmesh/webhook/receive/storage/HookConfigOperationManager.java
Outdated
Show resolved
Hide resolved
…used in UpdateWebHookConfigHandler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
… in dashboard) (apache#4272) * doc: Add JavaDoc for ClientManageController * doc: Add JavaDoc for HttpHandlerManager * doc: Add JavaDoc for AbstractHttpHandler * doc: Add JavaDoc for ConfigurationHandler * doc: Add JavaDoc for MetricsHandler * doc: Add JavaDoc for RegistryHandler * doc: Add JavaDoc for TopicHandler * doc: Add JavaDoc for EventHandler * doc: Add JavaDoc for TCPClientHandler * doc: Add JavaDoc for HTTPClientHandler * doc: Add JavaDoc for GrpcClientHandler * doc: Add JavaDoc for HookConfigOperationManager (Outside) * doc: Add JavaDoc for InsertWebHookConfigHandler * doc: Add JavaDoc for UpdateWebHookConfigHandler * doc: Add JavaDoc for DeleteWebHookConfigHandler * doc: Update JavaDoc for QueryWebHookConfigByIdHandler (WIP) * Breaking Change: Update JavaDoc for HookConfigOperationManager * Breaking Change: Update JavaDoc for QueryWebHookConfigByIdHandler * doc: Update JavaDoc for InsertWebHookConfigHandler * doc: Update JavaDoc for UpdateWebHookConfigHandler * doc: Update JavaDoc for DeleteWebHookConfigHandler * doc: Add JavaDoc for QueryWebHookConfigByManufacturerHandler * doc: Add JavaDoc for QueryRecommendEventMeshHandler * doc: Add JavaDoc for RedirectClientByIpPortHandler * doc: Add JavaDoc for RedirectClientByPathHandler and multiple minor changes * doc: Add JavaDoc for RedirectClientBySubSystemHandler * doc: Add JavaDoc for RejectClientByIpPortHandler * doc: Add JavaDoc for RejectClientBySubSystemHandler * doc: Add JavaDoc for RejectAllClientHandler * doc: Add JavaDoc for ShowClientBySystemHandler * doc: Add JavaDoc for ShowListenClientByTopicHandler and multiple minor changes * doc: Add JavaDoc for ShowClientHandler and multiple minor changes * fix: Remove redundant throw and its JavaDoc * fix: redundant import and formatting * feat: Streamline and Reduce existing single-line comments * Optimize: Explain more clearly why HookConfigOperationManager is not used in UpdateWebHookConfigHandler * Optimze: Explain HookConfigOperationManager more clearly in WebHookConfigOperation * Optimize: Concise the implement of handle() * Optimze: Remove some relatively simple comments * Fix: CI report reference not found
Fixes #4211.
Motivation
Please see #4211.
Modifications
Write comments for classes and methods in the
org.apache.eventmesh.runtime.admin
package which haven't been integrated in the eventmesh-dashboard.Some webhook related classes are also commented in order to provide help for others to understand, such as the logic of which implementation to use for
WebHookConfigOperation
interface.Example:
No changes are made in codes except the redundant
toString()
is removed inRedirectClientByIpPortHandler
.Documentation