Skip to content

Conversation

@harmim
Copy link
Contributor

@harmim harmim commented Sep 5, 2018

  • new feature
  • BC break? yes

Added support for repeatable --coverage-src option. It is useful when you have got eg. multiple directories in root of your project which you want to generate coverage for.

@milo
Copy link
Member

milo commented Sep 6, 2018

This PR looks great!
Solves #232, partially solves #225

@harmim
Copy link
Contributor Author

harmim commented Sep 8, 2018

So is it ready to be merged?

@milo
Copy link
Member

milo commented Sep 9, 2018

Not yet. IMHO one thing should be done differently but I have to try it.

}

$light = $total ? $total < 5 : count(file($entry)) < 50;
$nameReplacePairs = [];
Copy link
Member

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.

Copy link
Contributor Author

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?

Copy link
Member

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();
Copy link
Member

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;
Copy link
Member

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);
}
Copy link
Member

@milo milo Sep 10, 2018

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.....
);

@milo
Copy link
Member

milo commented Sep 10, 2018

Sorry if you see any mess here. I'm trying the review and resolve/unresolve conversation functionality.

@harmim harmim force-pushed the harmim-coverage-src-repetable branch from 9de88a3 to f89cab0 Compare September 11, 2018 09:15
@harmim
Copy link
Contributor Author

harmim commented Sep 11, 2018

@milo Thanks for your review. I have edited it.

milo pushed a commit that referenced this pull request Sep 11, 2018
@milo
Copy link
Member

milo commented Sep 11, 2018

Merged 2286131. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants