fix: correct relative time format for yesterday#1594
Conversation
|
Hi @MyLeeJiEun. Thanks for your PR. 😃 |
|
Hi @MyLeeJiEun. Thanks for your PR. I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts ICU relative time formatting for the "yesterday" case by switching to absolute-day formatting and aligning the formatter call with the correct overload, removing an unnecessary numeric argument. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Since this path now relies on
UDAT_ABSOLUTE_DAY, consider adding a short comment explaining whyUDAT_DIRECTION_LAST + UDAT_RELATIVE_DAYSwas incorrect here and whyUDAT_ABSOLUTE_DAYis required, to help future maintainers understand the semantic difference. - Double-check other branches of
formatRelativeTime(e.g., for today/tomorrow) to ensure they consistently use the appropriate ICU absolute vs relative units so that behavior doesn’t diverge subtly between similar cases.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since this path now relies on `UDAT_ABSOLUTE_DAY`, consider adding a short comment explaining why `UDAT_DIRECTION_LAST + UDAT_RELATIVE_DAYS` was incorrect here and why `UDAT_ABSOLUTE_DAY` is required, to help future maintainers understand the semantic difference.
- Double-check other branches of `formatRelativeTime` (e.g., for today/tomorrow) to ensure they consistently use the appropriate ICU absolute vs relative units so that behavior doesn’t diverge subtly between similar cases.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, MyLeeJiEun The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
1. Use UDAT_ABSOLUTE_DAY instead of UDAT_DIRECTION_LAST + UDAT_RELATIVE_DAYS 2. Remove the redundant numeric argument that was passed to formatter->format() 3. Ensure the ICU API call matches the correct overload for absolute day formatting Log: Fix yesterday's relative time formatting by using correct ICU API arguments fix: 修复"昨天"相对时间格式错误 1. 使用 UDAT_ABSOLUTE_DAY 替换 UDAT_DIRECTION_LAST + UDAT_RELATIVE_DAYS 2. 移除传递给 formatter->format() 的多余数值参数 3. 确保 ICU API 调用匹配绝对日期格式的正确重载 Log: 修复昨天相对时间格式化问题,使用正确的 ICU API 参数 PMS: BUG-359117
8d31e9c to
2ca16c2
Compare
|
/merge |
|
This pr cannot be merged! (status: blocked) |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
Log: Fix yesterday's relative time formatting by using correct ICU API arguments
fix: 修复"昨天"相对时间格式错误
Log: 修复昨天相对时间格式化问题,使用正确的 ICU API 参数
PMS: BUG-359117
Summary by Sourcery
Bug Fixes: