-
Notifications
You must be signed in to change notification settings - Fork 79
recursively set indentations for child arrays #164
Conversation
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.
Please add relevant tests to
class ValueGeneratorTest extends TestCase |
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.
sorry I dont have php7 on this machine, so I just added methods in same style.
* | ||
* @return array | ||
*/ | ||
public function complexArrayWCustomIndent() |
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.
Please use : array
/** | ||
* Data provider for testPropertyDefaultValueCanHandleComplexArrayWCustomIndentOfTypes test | ||
* | ||
* @return array |
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.
Can be dropped
* @param array $value | ||
* @param string $expected | ||
*/ | ||
public function testPropertyDefaultValueCanHandleComplexArrayWCustomIndentOfTypes($type, array $value, $expected) |
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.
: void
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.
Please add type declarations to the parameters: drop them from the docblock
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.
I was copy that code from complexArray test.
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.
sorry, forgot : void
Check the failures, something is not working as expected: https://travis-ci.org/zendframework/zend-code/jobs/445104487#L653 |
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.
sublime trim_trailing_white_space_on_save on 280 line :)
CS checks still fail: https://travis-ci.org/zendframework/zend-code/jobs/445361931#L675-L744 |
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.
once again
Last one: https://travis-ci.org/zendframework/zend-code/jobs/445367598#L675-L680 Maybe one level of nesting less would suffice 👍 |
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.
LGTM, thanks! 👍
PR for #163
I was use auto generated complex arrays with personal data for tests, so I cant publish it.