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.

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

replace Pipe with pipeline function #6

Merged
merged 5 commits into from
Jan 20, 2017
Merged

replace Pipe with pipeline function #6

merged 5 commits into from
Jan 20, 2017

Conversation

prolic
Copy link
Member

@prolic prolic commented Jan 20, 2017

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling ab7135c on pipeline into bbf2465 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 3815230 on pipeline into bbf2465 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 6690f47 on pipeline into bbf2465 on master.

@@ -42,7 +42,7 @@ public function state(): array
return $this->state;
}

private static function assertEvent(Message $event)
private function assertEvent(Message $event)
Copy link
Member

Choose a reason for hiding this comment

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

missing return type hint

{
array_unshift($secondCallback, $firstCallback);

return function ($value = null) use ($firstCallback, $secondCallback) {
Copy link
Member

Choose a reason for hiding this comment

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

$firstCallback is not used in closure

};
}

const pipeline = 'Prooph\Micro\Kernel\pipeline';

function pipleline(callable $firstCallback, callable ...$secondCallback): callable
Copy link
Member

Choose a reason for hiding this comment

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

$secondCallback should be renamed to $callbacks.

@codeliner
Copy link
Member

ah the comments by @oqq are good, @prolic ok from my side. If you make @oqq happy you can merge then ;)

@prolic
Copy link
Member Author

prolic commented Jan 20, 2017

done

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 4ff762a on pipeline into bbf2465 on master.

@prolic prolic merged commit 5d09e8c into master Jan 20, 2017
@prolic prolic deleted the pipeline branch January 20, 2017 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants