Skip to content
This repository was archived by the owner on Jan 8, 2020. It is now read-only.

Update library/Zend/Di/Definition/ClassDefinition.php#2265

Closed
noopable wants to merge 6 commits into
zendframework:masterfrom
noopable:patch-1
Closed

Update library/Zend/Di/Definition/ClassDefinition.php#2265
noopable wants to merge 6 commits into
zendframework:masterfrom
noopable:patch-1

Conversation

@noopable

Copy link
Copy Markdown
Contributor

@travisbot

Copy link
Copy Markdown

This pull request fails (merged 43dfe944 into 89eaf41).

@travisbot

Copy link
Copy Markdown

This pull request passes (merged 43dfe944 into 89eaf41).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please follow coding standards; all conditionals require braces:

if ($this->class !== $class) {
    return array();
}

@weierophinney

Copy link
Copy Markdown
Member

Also, please provide unit tests -- I have no idea what this is supposed to accomplish, and can't be assured it's not breaking other code.

@noopable

Copy link
Copy Markdown
Contributor Author

thanks.

I can't find unit tests about DefinitionList and ClassDefinition.
Should I make it? If so,please give me some time.

@alrik

alrik commented Aug 30, 2012

Copy link
Copy Markdown
Contributor

Well: ZendTest\Di\Definition\ClassDefinitionTest - here you go ;)

@travisbot

Copy link
Copy Markdown

This pull request fails (merged 7904415f into 89eaf41).

@travisbot

Copy link
Copy Markdown

This pull request fails (merged 40526b00 into 89eaf41).

@travisbot

Copy link
Copy Markdown

This pull request fails (merged 4c733119 into 89eaf41).

@noopable

Copy link
Copy Markdown
Contributor Author

I thought about it . I made a test in DefinitionListTest in this commit
I separated to another commit and a pull request.
#2277
ClassDefinitionTest is in it.

And I wrote reproductive code in http://framework.zend.com/issues/browse/ZF2-509?focusedCommentId=51761#comment-51761

Please check it .

@travisbot

Copy link
Copy Markdown

This pull request fails (merged 5840d0ca into 2db58cf).

@travisbot

Copy link
Copy Markdown

This pull request fails (merged 9e62962 into 2db58cf).

@noopable noopable closed this Sep 5, 2012
@noopable

noopable commented Sep 5, 2012

Copy link
Copy Markdown
Contributor Author

moved :
#2295

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants