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.

Sign up for GitHub

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

Jump to bottom

Filter out statements not in source roots #167

Merged
merged 1 commit into from
Jun 8, 2016
Merged

Filter out statements not in source roots #167

merged 1 commit into from
Jun 8, 2016

Conversation

pbatko
Copy link
Contributor

@pbatko pbatko commented May 12, 2016

Fixes #165

Adds a flag to scoverage.report.BaseReportWriter to optionally filter out statements from sources not in root sources.

@sksamuel sksamuel merged commit 8008068 into scoverage:master Jun 8, 2016
@gslowikowski
Copy link
Member

This PR breaks sbt-scoverage.

I would like to revert it, because I still have no answer to my question #165 (comment) what is it for.

@gslowikowski gslowikowski mentioned this pull request Jun 17, 2016
Merged
@sksamuel
Copy link
Member

sbt-scoverage just needs to be updated to set ignoreStatementsNotInSrcDirs: Boolean. Or this should have a default value.

@gslowikowski
Copy link
Member

So we have a mess here and I still don't know, what is it for.

@gslowikowski
Copy link
Member

Additionally, I would like to be able to use this version with older versions of Maven plugin. We should keep things backward compatible.

@sksamuel
Copy link
Member

Agreed. We need clarification from the original author what this was meant
to fix.

On 17 June 2016 at 11:56, Grzegorz Slowikowski [email protected]
wrote:

Additionally, I would like to be able to use this version with older
versions of Maven plugin. We should keep things backward compatible.

—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#167 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAtZGsCkf_fcOo9oAL3RxwlFBdxzGj8Nks5qMn1egaJpZM4IdA-5
.

@pbatko
Copy link
Contributor Author

pbatko commented Jun 21, 2016 •
edited
Loading

Adding regular Scala default value to that flag would work only in sbt, but not in Gradle and probably similarly not in Maven. Instead we can add another constructor that will itself set the default value, or use setters. WDYT? Adding another constructor seems to be the most straightforward.

Also, replied in another thread.

@pbatko
Copy link
Contributor Author

pbatko commented Jun 22, 2016

Here's PR to get compat back: #169

@gslowikowski gslowikowski mentioned this pull request Jul 29, 2016
Closed
gslowikowski added a commit to gslowikowski/scalac-scoverage-plugin that referenced this pull request Sep 17, 2016
@gslowikowski
Revert changes coming from scoverage#167 and scoverage#169.
c973752 
These PRs fix scoverage#165.

It's unclear what the problem really is. Issue author has "No source root found for ${sourcepath}" exception in Gradle
project. We cannot reproduce the problem. This must be something with user's project configuration or a bug in Gradle plugin.

The two PRs were merged hastily all will be reverted.

@pbatko please provide test project and report problem in [Gradle Scoverage plugin](https://github.com/scoverage/gradle-scoverage/issues) first.
gslowikowski added a commit that referenced this pull request Sep 17, 2016
@gslowikowski
Merge pull request #180 from gslowikowski/revert-pr-167-and-169
d764e15 
Revert changes coming from #167 and #169.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pbatko @gslowikowski @sksamuel