-
Notifications
You must be signed in to change notification settings - Fork 485
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
优化biz信息显示 #897
优化biz信息显示 #897
Conversation
# Conflicts: # sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/model/BizModel.java
WalkthroughThe recent updates focus on enhancing the representation of state history within the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 2
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (2)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/model/BizModel.java (1 hunks)
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/model/BizInfo.java (1 hunks)
Additional Context Used
@@ -147,7 +147,7 @@ public BizStateRecord(Date changeTime, BizState state) { | |||
@Override | |||
public String toString() { | |||
String date = sdf.format(changeTime); | |||
return String.format("state change to %s at %s", state, date); | |||
return String.format("state at %s to %s", date, state); |
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.
The change in the order of state and date in the toString
method of the BizStateRecord
class aligns with the PR's objective to enhance readability. However, it would be beneficial to add a comment explaining the rationale behind this change for future maintainers.
+ // Reordered to improve readability by showing the date before the state
return String.format("state at %s to %s", date, state);
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
return String.format("state at %s to %s", date, state); | |
// Reordered to improve readability by showing the date before the state | |
return String.format("state at %s to %s", date, state); |
return "Ark Biz: " + getIdentity() + ",\n classloader: " + classLoader | ||
+ ",\n current state: " + bizState + ",\n history states: " + bizStateChangeLogs; |
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.
The toString
method references bizStateChangeLogs
, which is not defined within the provided code snippet. Ensure that bizStateChangeLogs
is properly defined and initialized to avoid potential null reference issues and to accurately represent the state history.
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.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (1)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/model/BizModel.java (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/model/BizModel.java
Additional Context Used
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #897 +/- ##
============================================
+ Coverage 78.72% 78.86% +0.14%
Complexity 837 837
============================================
Files 164 164
Lines 6796 6796
Branches 1009 1009
============================================
+ Hits 5350 5360 +10
+ Misses 923 912 -11
- Partials 523 524 +1 ☔ View full report in Codecov by Sentry. |
@@ -147,7 +147,7 @@ public BizStateRecord(Date changeTime, BizState state) { | |||
@Override | |||
public String toString() { | |||
String date = sdf.format(changeTime); | |||
return String.format("state change to %s at %s", state, date); | |||
return String.format("state at %s to %s", date, state); |
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.
不是换成时间:state 吗?
String.format("%s -> %s", date, state);
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.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (2)
- sofa-ark-parent/core-impl/container/src/test/java/com/alipay/sofa/ark/container/model/BizModelTest.java (1 hunks)
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/model/BizInfo.java (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/model/BizInfo.java
Additional Context Used
Additional comments not posted (4)
sofa-ark-parent/core-impl/container/src/test/java/com/alipay/sofa/ark/container/model/BizModelTest.java (4)
67-67
: The assertion checks for the presence of "-> resolved" in thetoString
output, aligning with the PR's objective to enhance the presentation of state transitions. Ensure that thetoString
method inBizModel
is indeed updated to reflect this new format.
72-73
: These assertions verify the state transition from "resolved" to "activated". It's good practice to ensure that thetoString
method's output is consistent and accurately reflects the state transitions.
78-80
: Assertions here check for a sequence of state transitions up to "deactivated". This consistency in testing thetoString
output for each state transition is crucial for validating the PR's enhancements.
84-87
: The final set of assertions covers the entire sequence of state transitions, including the transition to "unresolved". This thorough testing ensures that thetoString
method's enhancements are correctly implemented across all state changes.
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
* 优化显示 * fix as bizStateRecords * 更新显示 (cherry picked from commit 1bf4217)
* 优化显示 * fix as bizStateRecords * 更新显示 (cherry picked from commit 1bf4217)
Summary by CodeRabbit
toString()
method inBizModel
to improve the representation of state history by changing terminology.toString
method inBizStateRecord
to enhance clarity by changing the order of state and date.