-
-
Notifications
You must be signed in to change notification settings - Fork 319
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
chore: rm useless code #900
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough该拉取请求对 Changes
Assessment against linked issues
Possibly related PRs
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
src/generate/dayjs.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /.eslintrc.js
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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: 0
🧹 Outside diff range and nitpick comments (1)
src/generate/dayjs.ts (1)
103-109
: 代码修改符合预期!移除格式不匹配的警告提示是合理的,因为:
- 在用户输入过程中,格式不匹配是预期的正常现象
- 这解决了 issue #51839 中提到的问题
建议在代码注释中补充以下信息:
- 移除此警告的具体原因
- 对最终用户体验的影响
/* istanbul ignore next */ const parseNoMatchNotice = () => { // zombieJ: // When user typing, its always miss match format. // This check is meaningless. + // 原因: + // 1. 在用户输入过程中,格式不匹配是正常的 + // 2. 在使用 mask 模式时,预填充内容也会导致不匹配 + // 影响: + // - 移除此警告可以减少控制台噪音 + // - 不会影响实际的日期解析功能 // https://github.com/ant-design/ant-design/issues/51839 // noteOnce(false, 'Not match any format. Please help to fire a issue about this.'); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/generate/dayjs.ts
(1 hunks)
🔇 Additional comments (1)
src/generate/dayjs.ts (1)
Line range hint 165-167
: 验证解析失败处理的影响
parseNoMatchNotice
函数在解析失败时被调用的两个位置是合适的:
- 周数解析失败时
- 常规格式解析失败时
Also applies to: 188-191
✅ Verification successful
解析失败处理逻辑验证完成,确认安全
根据代码分析和测试用例验证:
parseNoMatchNotice
函数仅在src/generate/dayjs.ts
文件中被调用,用于处理周数解析和常规格式解析失败的情况- 测试用例
tests/keyboard.spec.tsx
和tests/generate.spec.tsx
中包含了格式不匹配的测试场景 - 移除警告提示不会影响解析逻辑的正常工作流程
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否有其他地方调用了 parseNoMatchNotice
rg "parseNoMatchNotice" --type ts
# 检查相关测试用例
rg "parse.*format" --type test
Length of output: 258
Script:
#!/bin/bash
# 检查测试文件中的相关用例
rg "parse.*format" -g "*.test.*" -g "*.spec.*"
# 查看 parseNoMatchNotice 的完整上下文
rg -B 5 -A 5 "parseNoMatchNotice" src/generate/dayjs.ts
# 检查整个文件中的解析逻辑
ast-grep --pattern 'function parse($_) { $$$ }'
Length of output: 1726
@@ -100,9 +99,13 @@ const parseLocale = (locale: string) => { | |||
return mapLocale || locale.split('_')[0]; | |||
}; | |||
|
|||
/* istanbul ignore next */ | |||
const parseNoMatchNotice = () => { |
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.
这是个空方法,删掉?
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.
这里没删是留着写原因的,否则万一以后遇到格式不匹配又会再写一遍。大家来回捯饬。
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #900 +/- ##
==========================================
- Coverage 95.46% 95.46% -0.01%
==========================================
Files 64 64
Lines 2734 2733 -1
Branches 742 770 +28
==========================================
- Hits 2610 2609 -1
Misses 121 121
Partials 3 3 ☔ View full report in Codecov by Sentry. |
close ant-design/ant-design#51839
看了一下代码,parse 逻辑在用户输入中是持续校验的。但是问题是,输入中不一定会匹配格式,导致无效的 warning 出来。而 mask 模式下,由于会预先填充内容,导致它一定是匹配不正确的。
这个 parse 代码存在 5 年了,一直没遇到有效的反馈。应该是安全删除的。
cc @iamkun
Summary by CodeRabbit