-
-
Notifications
You must be signed in to change notification settings - Fork 73
Repeatable --coverage-src option #388
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
Conversation
|
So is it ready to be merged? |
|
Not yet. IMHO one thing should be done differently but I have to try it. |
| } | ||
|
|
||
| $light = $total ? $total < 5 : count(file($entry)) < 50; | ||
| $nameReplacePairs = []; |
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.
There should be single replacement, common path of all sources. Otherway duplicates may appear.
And it can be done before foreach loop.
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.
@milo What is the easiest way to find common path of all sources?
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.
This code can be reused.
| $iterator = is_dir($this->source) | ||
| ? new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($this->source)) | ||
| : new \ArrayIterator([new \SplFileInfo($this->source)]); | ||
| $iterator = new \AppendIterator(); |
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.
No brackets according to CS consistency.
| } | ||
| } | ||
| $source = dirname($source . 'x'); | ||
| $sources[] = $source; |
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.
$sources = [dirname(...)];| ? new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($source)) | ||
| : new \ArrayIterator([new \SplFileInfo($source)]); | ||
| $iterator->append($partialIterator); | ||
| } |
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.
Since $partialIterator is not used anywhere else, write it this way:
$iterator->append(
is_dir($source)
? new \Recursive.....,
: new \Array.....
);|
Sorry if you see any mess here. I'm trying the review and resolve/unresolve conversation functionality. |
9de88a3 to
f89cab0
Compare
|
@milo Thanks for your review. I have edited it. |
|
Merged 2286131. Thank you! |
Added support for repeatable
--coverage-srcoption. It is useful when you have got eg. multiple directories in root of your project which you want to generate coverage for.