[WIP] Support for multiple reporters #202
Conversation
Current coverage is 97.31% (diff: 100%)@@ master #202 diff @@
==========================================
Files 21 22 +1
Lines 736 781 +45
Methods 180 192 +12
Messages 0 0
Branches 0 0
==========================================
+ Hits 715 760 +45
Misses 21 21
Partials 0 0
|
ezzatron
force-pushed
the
multiple-reporters
branch
3 times, most recently
from
3f99d15 to
edf6b0c
Compare
There was a problem hiding this comment.
@ezzatron looks really good! An improvement for sure. Just had one comment/question.
With regards to the env variables - env variables seem useful for plugins. I think I added most of that functionality to support https://github.com/peridot-php/peridot-concurrency. I would be curious if the new functionality works with that plugin (or if that plugin still works in general) - not a huge deal as I have not used it in some time, and I don't think many people are.
| }, $this->reporters); | ||
| } | ||
|
|
||
| private $reporters; |
There was a problem hiding this comment.
Would you mind bumping this guy up to the top? I think that is consistent with the rest of the code right? If it's a preferred style thing I am definitely open to talking about it.
There was a problem hiding this comment.
Yeah, no worries man. I'll sort that out.
| } | ||
|
|
||
| throw new \RuntimeException("Reporter class could not be created"); | ||
| return new CompositeReporter( | ||
| array_merge( |
|
For the record, it looks like a change broke the concurrency plugin. I am cool leaving it be for now. If someone opens an issue I will look into it. I don't think many people are using it anymore, so I'm not expecting a problem |
|
@brianium I tried to find a dependent package for the concurrency stuff so that I could try it out for myself, but there's only one listed in Packagist dependents that isn't How did you figure out that it broke? EDIT: Tried running the |
ezzatron
force-pushed
the
multiple-reporters
branch
from
edf6b0c to
42e2bb9
Compare
|
Yeah. I did the same thing. I think it has been broken for a while. I haven’t seen any issues coming in, so either people are using pinned versions - or it never quite took off :) You ok with punting on it for a while? Concurrency might be a cool feature to add to the core library at some point. |
|
@brianium Yeah, I'm happy to let it go for now. At some point I probably need to go through all the repos and do some maintenance anyway. Hopefully I can get it working when I do that. |
|
I also have some docs for this on a branch of the website: peridot-php/peridot-php.github.io@d15ab49 |
This PR adds support for multiple reporters.
Summary/justification of changes:
reportercommand-line option inperidot.php.--reporteroptions.runner.endevent fires, any buffered output is displayed after the primary reporter's output, separated by a single newline. This prevents interlaced output, while retaining streaming output for the primary reporter.getReporter()andsetReporter().PERIDOT_REPORTERenvironment variable is used for, but I also retained support for it, and added a similarPERIDOT_REPORTERSenvironment variable, which contains all the reporter names, separated by comma.Closes #186.