Change in assignment to scalacOptions key ignores any build specific options

[seveneves]

The problem is with following code in io.github.davidgregory084.TpolecatPlugin in latest 0.2.2 version.

  override def projectSettings: Seq[Setting[_]] = Seq(
    Def.derive(
      scalacOptions := scalacOptionsFor(scalaVersion.value, tpolecatScalacOptions.value)
    ),

and scalacOptions is now set a value via := instead of being added ++= as it was done in version 0.1.22 for example.

This results in ignoring any user specified scalacOptions in ThisBuild scope.

ThisBuild / scalacOptions ++= Seq(...)

Is there a reason of not using ++= instead of :=?

[DavidGregory084]

No, there's no reason for it @seveneves, it's just an oversight.

As of the changes in 0.2.x it's probably better to manipulate tpolecatScalacOptions instead.

I will fix & cut a new release Monday 👍

[seveneves]

it's probably better to manipulate tpolecatScalacOptions instead.

That was my initial intention for this issue but then I need to use the plugin's ADTs instead of simple strings. For example,

ThisBuild / tpolecatScalacOptions += new ScalacOption(List("-Xsource:3"), _ > ScalaVersion.V2_13_0)

It is much longer version of just

ThisBuild / scalacOptions += "-Xsource:3"

I will fix & cut a new release Monday 👍

That would be great! Thanks

[DavidGregory084]

@seveneves out of curiosity is there a reason to use scope delegation from ThisBuild to set a project-level setting (scalacOptions)? ThisBuild is usually used for build-level stuff like whether code coverage is enabled.

I think this will always be overridden by any project-specific usage of that setting from any plugin because of the scope delegation rules.

Rule 4: Given a scope, delegate scopes are searched by substituting the subproject axis in the following order: the given subproject, ThisBuild, and then Zero.

I think it's more common to have a commonSettings that is applied to every subproject, e.g. a recent version of this I saw was in scalacache.

[seveneves]

Thanks for accepting the PR!

I think it's more common to have a commonSettings

This would be the way to fix it also. But we have about 100+ git repos that use ThisBuild / scalacOptions with different options added. Some of them can have 10 sub-projects. It definitely needs a clean up and moving such settings to a common plugin/settings.

be overridden by any project-specific usage

yes, if it uses := and not ++=.

Rule 4

Thanks for sharing!

[mihaisoloi]

@DavidGregory084 I don't think this fix is released, should it be part of 0.2.3?

[seveneves]

@DavidGregory084 I think this might have broken Test / console / scalacOptions.

I will check how it can be fixed and will create a PR

[DavidGregory084]

Hmm @seveneves I am struggling with this one a bit. I don't think that we can continue to support inheriting ThisBuild's scalacOptions.

The reason is that it collides with the ScalacOptions DSL in an unfortunate way:

By inheriting existing scalacOptions, we cannot configure options using just the tpolecatScalacOptions key - we have to try and filter out tpolecatExcludeOptions from an existing set of scalacOptions that has already been turned into Seq[String]. This causes problems like #74 where we have to figure out how to deal with multiple token options for filtering.

If we fill out the ScalacOptions DSL with the options that are commonly used in your repos, would it make it easier for you to drop this usage of ThisBuild / scalacOptions?

One thing I am considering is whether it would make sense to write a scalafix migration to migrate from scalacOptions to the ScalacOptions DSL.

[seveneves]

@DavidGregory084 That's fine. Let's drop support for ThisBuid. Thanks for letting me know

[DavidGregory084]

Sorry for the trouble - if you can think of any ideas as to how we can support it I'm happy to give it a go!