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

add defaultColor #704

Merged
merged 2 commits into from
May 18, 2017
Merged

add defaultColor #704

merged 2 commits into from
May 18, 2017

Conversation

damonto
Copy link

@damonto damonto commented May 17, 2017

@overtrue
Copy link
Collaborator

测试过了？有效？

@damonto
Copy link
Author

damonto commented May 18, 2017

@overtrue 测试过了 可用.

mingyoung
mingyoung reviewed May 18, 2017
View reviewed changes
src/Notice/Notice.php Outdated
*
* @return this
*/
public function defaultColor($color)
Copy link
Collaborator

Choose a reason for hiding this comment

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

方法直接命名为 color 如何？
public function color($color) {}

Copy link
Author

Choose a reason for hiding this comment

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

可以，但是现有的文档中写的 defaultColor 如果改为 Color 那么就代表需要修改文档，并且之前用这个方法需要改为 color

Copy link
Collaborator

Choose a reason for hiding this comment

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

我以为这是个新增的方法 🤣
那保留这样吧

@mingyoung mingyoung merged commit 5f79f05 into w7corp:master May 18, 2017
@damonto damonto mentioned this pull request May 18, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mingyoung mingyoung mingyoung left review comments

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

Successfully merging this pull request may close these issues.
3 participants
@damonto @overtrue @mingyoung