Fix withArgs method when an empty array passed. Issue #278 #279
Conversation
| @@ -327,6 +327,7 @@ public function with() | |||
| */ | |||
| public function withArgs(array $args) | |||
| { | |||
| if(empty($args)) return $this->withNoArgs(); | |||
There was a problem hiding this comment.
Please reformat the code, so { and } are used too. It can be easily done with PhpStorm's "Reformat Code" feature.
|
@davedevelopment, build failed, but only for |
| @@ -315,7 +316,11 @@ protected function _matchArg($expected, &$actual) | |||
| */ | |||
| public function with() | |||
| { | |||
| $this->_expectedArgs = func_get_args(); | |||
| $args = func_get_args(); | |||
| if (empty($args)) { | |||
There was a problem hiding this comment.
This is code duplication. Now the with method can be transformed into return $this->withArgs(func_get_args()) safely.
|
Can you squash commits now? |
| @@ -339,7 +342,8 @@ public function withArgs(array $args) | |||
| public function withNoArgs() | |||
| { | |||
| $this->_noArgsExpectation = true; | |||
There was a problem hiding this comment.
Hopefully nobody will do this, but if you call ->withNoArgs()->with('arg') then the $this->_noArgsExpectation flag won't be reset. Maybe we should reset that flag to false when passing in valid argument list?
Is it ok, @davedevelopment ?
There was a problem hiding this comment.
So I'm guessing such type of protection isn't needed.
There was a problem hiding this comment.
I was under the impression it was fixed by 833f511
There was a problem hiding this comment.
Indeed it was. Never saw that. I personally always reply to inline code comments when I implement wasn't been asked there. This way it's easier.
Always doing a squash before PR merging is good idea, since having 5 commits in main repo just to change 2 small methods doesn't make commit history cleaner.
There was a problem hiding this comment.
Squashing doesn't really interest me, unless the history is really messy.
We have merge commits, they're good enough for me.
On 25 Feb 2014 12:28, "Alexander Obuhovich" notifications@github.com
wrote:
In library/Mockery/Expectation.php:
@@ -339,7 +342,8 @@ public function withArgs(array $args)
public function withNoArgs()
{
$this->_noArgsExpectation = true;It was. Never saw that. I personally always reply to inline code comments
when I implement wasn't been asked there. This way it's easier.Always doing a squash before PR merging is good idea, since having 5
commits in main repo just to change 2 small methods doesn't make commit
history cleaner.—
Reply to this email directly or view it on GitHubhttps://github.com//pull/279/files#r10033466
.
Fix withArgs method when an empty array passed. Issue #278
Issue #278