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

Make Scaladoc (but not Unidoc) warnings non-fatal #787

Merged

Conversation

travisbrown
Copy link
Contributor

This change addresses the errors mentioned here.

The issue is that Unidoc supports cross-module links, but Scaladoc does not. Since we're using Unidoc, it's reasonable to want to be able to have links between modules, but the publish task will still use Scaladoc to create the docs for the javadoc.jar artifacts. Any cross-module links will result in warnings, which will currently fail the build.

This change makes all warnings non-fatal for Scaladoc, but then turns fatal warnings back on for Unidoc. This allows publishing to succeed even with cross-module link warnings from Scaladoc, but the build will still fail if Unidoc runs into warnings.

An alternative solution would be to use publishArtifact in (Compile, packageDoc) := false to skip the javadoc.jar artifacts altogether, but that could cause other problems for repositories that expect these.

Note that there currently aren't any cross-module links in the API docs, but it's a reasonable thing to want to support even outside of the context of #786 (where the errors are first turning up).

@ghost
Copy link

ghost commented Jan 7, 2016

@travisbrown I really cannot see a better way 👍

Any chance, please, that you could you also add to sbt-catalysts?

@@ -37,7 +37,8 @@ lazy val commonSettings = Seq(
compilerPlugin("org.scalamacros" %% "paradise" % "2.1.0-M5" cross CrossVersion.full),
compilerPlugin("org.spire-math" %% "kind-projector" % "0.6.3")
),
parallelExecution in Test := false
parallelExecution in Test := false,
scalacOptions in (Compile, doc) := (scalacOptions in (Compile, doc)).value.filter(_ != "-Xfatal-warnings")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scalacOptions in (Compile, doc) -= "-Xfatal-warnings"

TFW you teach Travis Brown something for once ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dwijnand Nice!—I had no idea that would work.

My approach to dealing with SBT is 95% cargo-cultery, and there's a similar filter a little further down in the file—I'll update both in this PR when I'm back at my desktop.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But not if this is added to sbt-catalsysts due to typesafe activator

😉

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@inthenow Does activator seriously not respect the sbt version in build.properties?

@dwijnand
Copy link
Contributor

dwijnand commented Jan 7, 2016

1 code cleanup change, but other than that 👍 from me, I'd've gone this way as well.

Looks like TravisCI needs a kick.

@codecov-io
Copy link

Current coverage is 88.48%

Merging #787 into master will not affect coverage as of 742ff97

@@            master    #787   diff @@
======================================
  Files          166     166       
  Stmts         2292    2292       
  Branches        75      75       
  Methods          0       0       
======================================
  Hit           2028    2028       
  Partial          0       0       
  Missed         264     264       

Review entire Coverage Diff as of 742ff97

Powered by Codecov. Updated on successful CI builds.

@ceedubs
Copy link
Contributor

ceedubs commented Jan 8, 2016

Fantastic. Thanks, @travisbrown!

ceedubs added a commit that referenced this pull request Jan 8, 2016
Make Scaladoc (but not Unidoc) warnings non-fatal
@ceedubs ceedubs merged commit a13f4f7 into typelevel:master Jan 8, 2016
@erik-stripe
Copy link
Contributor

👍

@erik-stripe
Copy link
Contributor

Whooops, was looking at an out-of-date page somehow. Great job everyone, thanks Travis!

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.

6 participants