Skip to content

Conversation

@yuxuan-ctrl
Copy link
Contributor

@yuxuan-ctrl yuxuan-ctrl commented May 26, 2023

What kind of change does this PR introduce(这个 PR 引入了什么样的变化)?

  • Bugfix(修正错误)
  • Feature(新功能)
  • Refactor(重构)
  • Build-related changes(与构建相关的更改)
  • Other, please describe(其他,请描述):

Does this PR introduce a breaking change(这次 PR 引入了一个重大变化吗)?

  • Yes(是)
  • No(否)

If yes, please describe the impact and migration path for existing applications(如果是,请描述现有应用程序的影响和迁移路径):

The PR fulfills these requirements(PR 符合以下要求)

  • All tests are passing(所有测试都通过)

Other information(其他信息)

修复了单元测试引用路径问题,个人觉得依赖外部插件作为翻译有风险,应当在最后的选项提供一个默认的(比如取中文首字母)的方案作为不报错的备用方案,否则在使用Vs-code插件的时候,排查错误比较困难。

@CLAassistant
Copy link

CLAassistant commented May 26, 2023

CLA assistant check
All committers have signed the CLA.

@yuxuan-ctrl
Copy link
Contributor Author

#377 关联Issue

getFullChars: function (str) {
let result = '',
name;
// let reg = new RegExp('[a-zA-Z0-9- ]');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

整理一下代码,没用的注释删掉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好的

for (let key in this.full_dict) {
if (-1 !== this.full_dict[key].indexOf(str)) {
return this._capitalize(key);
break;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return后面的break没用吧?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

是的,这个break无用

@yuxuan-ctrl
Copy link
Contributor Author

@z979054461 已去除无用注释,谢谢

@z979054461
Copy link
Collaborator

你单独提一下pinyin好了,单元测试我来修

@yuxuan-ctrl
Copy link
Contributor Author

@z979054461 已去除单元测试的修复,仅上次pinyin代码,谢谢

@z979054461 z979054461 merged commit 85dd72a into alibaba:master Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants