-
-
Notifications
You must be signed in to change notification settings - Fork 360
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
Add JavaModule.mandatoryJavacOptions
#3503
Conversation
This will hold options contributed by traits like `ErrorProneModule`. Those are not propagated to the inner test trait intentionally, since those options are typically configured by other means, e.g. dedicated targets.
I think the idea sounds reasonable. Why is it called In the past the convention was just that |
I though about Both could be changed to no propagate (their value should still compute to the same value, which is a good indicator) too, but unfortunately, this would be a binary incompatible change and has to wait for Mill For the sake of consistency, I can change to |
Changed to |
JavaModule.managedJavacOptions
JavaModule.mandatoryJavacOptions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah the name could be improved, maybe in 0.13.0 or something. Until then we'll just have to compensate by writing really good scaladoc so people who encounter it can read what it is for
What if we exposed a task in def javacOptionsTestsPropagation: Task[Seq[String] => Seq[String]] = T.task { identity } This way your plugin can override it to filter out the option that you don't want to propagate. |
I think filtering options isn't my favorite go-to solution, since it's always a bit error-prone. Since we use stackable traits which can't be removed we also should take care to not add unnecessary options, which can't be easily removed. The new |
Needed to add a MiMa exclusion, which should be safe in this case.
I took the opportunity to no longer propagate |
I checked, we're not propagating |
Not propagating mandatoryScalacOptions sounds fine. If the test module needs it they can inherit the trait (and often already do in cases like ScalaJs options or ScalaNative options) |
This will hold options contributed by traits like
ErrorProneModule
.Those are not propagated to the inner test traits intentionally, since those options are typically configured by other means, e.g. dedicated targets.
I also removed propagation of
ScalaModule.mandatoryScalacOptions
to its innerScalaTests
trait, for that same reasons and for consistence. Now allmandatory*
targets don't propagate their values to inner tests.