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.

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

Use unmanagedSourceDirectories for sourceDirectories #229

Merged
merged 1 commit into from
May 12, 2016

Conversation

fthomas
Copy link
Contributor

@fthomas fthomas commented May 12, 2016

sourceDirectories in hasScalafmt is currently initialized with
List(scalaSource.value) which means that scalafmt only looks into
one source directory per project. But a typical Scala.js cross
project has at least two source directories js/ and shared/ or
jvm/ and shared/. Sometimes there are also multiple source
directories for different Scala versions.

This change initializes sourceDirectories in hasScalafmt with
unmanagedSourceDirectories which contains all source directories
with manually created sources. In my tests
unmanagedSourceDirectories always includes scalaSource.

Here are for example both settings for typelevel/cats:

coreJVM/scalaSource
[info] cats/core/.jvm/src/main/scala

coreJVM/unmanagedSourceDirectories
[info] List(
cats/core/.jvm/src/main/scala-2.11,
cats/core/.jvm/src/main/scala,
cats/core/.jvm/src/main/java,
cats/core/src/main/scala)

And since cats has no sources in cats/core/.jvm/src/main/scala,
scalafmt won't format anything.

`sourceDirectories in hasScalafmt` is currently initialized with
`List(scalaSource.value)` which means that scalafmt only looks into
one source directory per project. But a typical Scala.js cross
project has at least two source directories js/ and shared/ or
jvm/ and shared/. Sometimes there are also multiple source
directories for different Scala versions.

This change initializes `sourceDirectories in hasScalafmt` with
`unmanagedSourceDirectories` which contains all source directories
with manually created sources. In my tests
`unmanagedSourceDirectories` always includes `scalaSource`.

Here are for example both settings for typelevel/cats:

> coreJVM/scalaSource
[info] cats/core/.jvm/src/main/scala

> coreJVM/unmanagedSourceDirectories
[info] List(
  cats/core/.jvm/src/main/scala-2.11,
  cats/core/.jvm/src/main/scala,
  cats/core/.jvm/src/main/java,
  cats/core/src/main/scala)

And since cats has no sources in cats/core/.jvm/src/main/scala,
scalafmt won't format anything.
@codecov-io
Copy link

codecov-io commented May 12, 2016

Current coverage is 90.57%

Merging #229 into master will not change coverage

@@             master       #229   diff @@
==========================================
  Files            30         30          
  Lines          1633       1633          
  Methods        1529       1529          
  Messages          0          0          
  Branches        104        104          
==========================================
  Hits           1479       1479          
  Misses          154        154          
  Partials          0          0          

Sunburst

Powered by Codecov. Last updated by 39aca53...c745e9f

@fthomas
Copy link
Contributor Author

fthomas commented May 12, 2016

I should mention that I've tested this with refined and it allowed to get rid of this workaround.

@olafurpg olafurpg merged commit 6423be5 into scalameta:master May 12, 2016
@olafurpg
Copy link
Member

From the sbt documentation:

sbt collects sources from unmanagedSourceDirectories, which by default consists of scalaSource and javaSource.

Nice. Thanks a lot @fthomas 😄

@fthomas
Copy link
Contributor Author

fthomas commented May 14, 2016

For the sake of completeness, this is how unmanagedSourceDirectories is defined by sbt by default:

unmanagedSourceDirectories := makeCrossSources(
  scalaSource.value, javaSource.value, scalaBinaryVersion.value, crossPaths.value)

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