Skip to content

Add excludedSymbols with reasonable defaults #178

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

Merged
merged 1 commit into from Sep 25, 2016
Merged

Add excludedSymbols with reasonable defaults #178

merged 1 commit into from Sep 25, 2016

Conversation

ghost
Copy link

@ghost ghost commented Sep 14, 2016

No description provided.

@ghost
Copy link
Author

ghost commented Sep 15, 2016

History: Whilst scala.js testing I got some compile time errors as some macros are indeed being instrumented. This is not new - eg this shapeless class

Reason is that the current check just sees if a method returns an Expr but often a Tree is returned, as in that shapeless example

The reason I hit this in the scala.js version is that when it tries to to write data at compile time, it only has an sjs writer, that won't work at compile time as that is still in the jvm world

So this PR adds an option to specify a list of symbols to ignore, and defaults to Expr and Tree. This also means previous behaviour can be retained.

Potentially, the comment in the code "todo add way of allowing methods that return Expr" can be partially done by removing the excluded symbol at a project build level .Not perfect, but a step in the right direction.

Full usage documentation to appear in sbt-scoverage as that is where users will look.

@ghost ghost closed this Sep 17, 2016
@ghost ghost reopened this Sep 17, 2016
@rorygraves
Copy link
Contributor

This looks good to me - lets merge it - over to @gslowikowski / @sksamuel

@gslowikowski gslowikowski merged commit cff95bd into scoverage:master Sep 25, 2016
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.

3 participants