-
Notifications
You must be signed in to change notification settings - Fork 27
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
add runner accessors to Command #123
add runner accessors to Command #123
Conversation
@austinsmorris can I get your thoughts on all the things mentioned here + a merge if things seem cool to you? |
*/ | ||
public function getRunner() | ||
{ | ||
return $this->runner; |
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.
If we can set any RunnerInterface, then we can return more than just Runner (fix typehint). Same holds true for the constructor. It can just be Runner Interface instead of Peridot\Runner\Runner.
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.
cool
Looks good, but why did the last commit drop coverage by 5%? |
@austinsmorris not sure on coverage details, as usual - hhvm coverage seems to be flawed. something with lib we are using? I definitely don't want that on the repo page. I'll look into it. |
@austinsmorris yeah, it looks like there is an open issue on hhvm coverage things, so I just reran the build and it put it back at 95. This should be good to merge. |
add runner accessors to Command
Fixes #121
Please note the intended 2.0 goal mentioned by #120 - once runner initialization is added to the command, we can have a default runner result of
Peridot\Runner\Runner
, similar to how the default loader mechanism works in the command.