Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: 通过环境变量, 禁用 copy 操作. #115

Merged
merged 1 commit into from
Jan 12, 2018
Merged

feat: 通过环境变量, 禁用 copy 操作. #115

merged 1 commit into from
Jan 12, 2018

Conversation

daya0576
Copy link

@daya0576 daya0576 commented Jan 11, 2018 •
edited by mozillazg
Loading

PR 描述

通过环境变量, 禁用 copy 操作 #113.

待办事项

  • 符合代码规范
  • 单元测试
  • 文档

@pep8speaks
Copy link

pep8speaks commented Jan 11, 2018 •
edited
Loading

Hello @daya0576! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on January 12, 2018 at 01:55 Hours UTC

@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 99.245% when pulling 809b7d5 on daya0576:PYPINYIN_NO_DICT_COPY into 974350d on mozillazg:develop.

1 similar comment
@coveralls
Copy link

coveralls commented Jan 11, 2018 •
edited
Loading

Coverage Status

Coverage increased (+0.004%) to 99.245% when pulling 809b7d5 on daya0576:PYPINYIN_NO_DICT_COPY into 974350d on mozillazg:develop.

mozillazg
mozillazg reviewed Jan 12, 2018
View reviewed changes
tests/_test_env.py Outdated
reload(pypinyin.constants)
assert pypinyin.constants.PINYIN_DICT is pypinyin.pinyin_dict.pinyin_dict


Copy link
Owner

Choose a reason for hiding this comment

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

这里多了个空行。

@mozillazg
Copy link
Owner

@daya0576 Thanks for your PR!

Please fix the pep8 issue:

Line 27:1: W391 blank line at end of file

@daya0576 daya0576 force-pushed the PYPINYIN_NO_DICT_COPY branch from 809b7d5 to e3c8f84 Compare January 12, 2018 01:55
@codecov
Copy link

codecov bot commented Jan 12, 2018 •
edited
Loading

Codecov Report

Merging #115 into develop will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #115      +/-   ##
===========================================
+ Coverage    99.24%   99.24%   +<.01%     
===========================================
  Files           20       20              
  Lines          527      530       +3     
===========================================
+ Hits           523      526       +3     
  Misses           4        4
Impacted Files Coverage Δ
pypinyin/constants.py 97.91% <100%> (+0.13%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 974350d...e3c8f84. Read the comment docs.

@daya0576
Copy link
Author

Fixed. XD

How do u run travis-ci test locally? I tried to use docker but failed.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 98.868% when pulling e3c8f84 on daya0576:PYPINYIN_NO_DICT_COPY into 974350d on mozillazg:develop.

@coveralls
Copy link

coveralls commented Jan 12, 2018 •
edited
Loading

Coverage Status

Coverage increased (+0.004%) to 99.245% when pulling e3c8f84 on daya0576:PYPINYIN_NO_DICT_COPY into 974350d on mozillazg:develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 98.868% when pulling e3c8f84 on daya0576:PYPINYIN_NO_DICT_COPY into 974350d on mozillazg:develop.

@daya0576
Copy link
Author

Looks all good. Is there anything else should I do to improve this PR? 🤩

mozillazg
mozillazg approved these changes Jan 12, 2018
View reviewed changes
Copy link
Owner

@mozillazg mozillazg left a comment

Choose a reason for hiding this comment

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

LGTM!

@mozillazg
Copy link
Owner

@daya0576

How do u run travis-ci test locally? I tried to use docker but failed.

Is there any error message?
You can use make test instead of tox -e $TOX_ENV, if you have problem about tox.

@mozillazg
Copy link
Owner

@bors-homu r+

@bors-homu
Copy link
Collaborator

📌 Commit e3c8f84 has been approved by mozillazg

bors-homu added a commit that referenced this pull request Jan 12, 2018
@bors-homu
Auto merge of #115 - daya0576:PYPINYIN_NO_DICT_COPY, r=mozillazg
eb9bd7d 
feat: 通过环境变量, 禁用 copy 操作.

## PR 描述
通过环境变量, 禁用 copy 操作.

## 待办事项

* [x] 符合代码规范
* [x] 单元测试
* [x] 文档

<!--
感谢你的贡献！❤️
-->
@bors-homu
Copy link
Collaborator

⌛ Testing commit e3c8f84 with merge eb9bd7d...

@bors-homu
Copy link
Collaborator

☀️ Test successful - status-travis
Approved by: mozillazg
Pushing eb9bd7d to develop...

@bors-homu bors-homu merged commit e3c8f84 into mozillazg:develop Jan 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mozillazg mozillazg mozillazg approved these changes

Assignees
No one assigned
Labels
new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@daya0576 @pep8speaks @coveralls @mozillazg @bors-homu