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 typelevel scalac options lib #54

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

davesmith00000
Copy link
Contributor

This PR attempts to swap the custom flags implementation for the one in scalac-options. Nobody has asked for this, but I've had to abandon mill-tpolecat as it is falling out of date, so I thought I'd have a go at helping bring it up to speed.

The PR is imperfect, I'll add some comments below. If there is any interest I'm happy to help make changes to get it over the line. If not, feel free to close. 🙂

This is the result of this chat and one or two other PRs. Just trying to be a good citizen. 😄

"-Xfatal-warnings",
"-Ykind-projector"
), s"scalacOptions did not match, got instead: ${project.scalacOptions()}")
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I expected this test to fail, but it didn't (locally), which leads me to believe it isn't running. How should I be running it?

@@ -5,6 +5,6 @@ import mill.scalalib._

trait TpolecatModule extends ScalaModule {
override def scalacOptions = T {
Tpolecat.scalacOptionsFor(scalaVersion())
super.scalacOptions() ++ Tpolecat.scalacOptionsFor(scalaVersion())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The other of the traits might mean - I think - that you could accidentally remove other settings previously set, if you don't append. But I could be wrong. This change is a separate commit and easy to revert.

@DavidGregory084
Copy link
Owner

Thanks for the PR @davesmith00000!

I'm happy to review the PR if you need, but looking at the discussions in other repos it seems like folks are gravitating toward using scalac-options for Mill builds, so maybe this project has outlived its usefulness.

Do you think that mill-tpolecat still provides some value as a wrapper for scalac-options? Or would it be better to signpost scalac-options in the README of this repo and archive this?

@DavidGregory084
Copy link
Owner

BTW if you have reach with Mill users, opinions about this decision one way or another would be welcome!

@davesmith00000
Copy link
Contributor Author

davesmith00000 commented Oct 14, 2024

Good morning @DavidGregory084,

I'll attempt to spread the link around and we'll see if anyone else has an opinion. Maybe give it a few days and then do as you see fit. 🙂

I think archiving it and sign-posting scalac-options is a valid strategy, particularly if you don't want to look after this thing. Until yesterday I didn't even know scalac-options was available, I was just struggling on.

My view is divided and probably not terribly helpful, but if it helps you reach a decision:

  1. I dislike that the Scala community is so sbt centric. Mill and Scala-CLI offer different but very good alternatives, and I'd like to see them be given first-class citizen status in general. Ok, sbt 2.0 is nearly upon us and that can only be a good thing, but I do feel we collectively have a degree of stockholme syndrome going on there, and it would be nice if we (all) stopped treating it like the knee-jerk default build tool. (Huge respect to sbt and the people who work on it, by the way! I just think it's based on one idea of how we can work, and that sometimes that approach is overkill when there are other valid approaches.)

  2. I personally am a bit of a caveman developer, and I only need one set of consistent flags across all my builds, preferably ones that cleverer / more interested people than me have worked out that I can steal. So scalac-options is good enough for me. But people I know who use sbt-tpolecat commercially make use of the ability to have more lax settings locally and stricter ones in their CI builds, and we lose that without mill-tpolecat. Then again, how many people are using Mill commerically? See point 1.

I'd like to see it stick around, better yet I'd like to see a tpolecat-everything project that publishes plugins for everything consistently, but if the burden isn't worth it then letting it die (and sign posting the alternative) is, of course, perfectly reasonable.

Either way, thank you for the project, I have enjoyed it and made good use of it till now!

@joan38
Copy link
Collaborator

joan38 commented Oct 14, 2024

I never did this PR because we just abandoned this plugin in favor of scalac-options directly:

  override def scalacOptions = super.scalacOptions() ++
    ScalacOptions.defaultTokensForVersion(ScalaVersion.unsafeFromString(scalaVersion())

And if you need to customize:

  override def scalacOptions = super.scalacOptions() ++ ScalacOptions.tokensForVersion(
    ScalaVersion.unsafeFromString(scalaVersion()),
    ScalacOptions.default + sourceFutureMigration ++ fatalWarningOptions
  )

So I'm not sure what value mill-tpolecat really adds but this change is the way to go for this plugin.

allScalacOptions.filter(_.isSupported(scalaVer)).flatMap(_.names)
}
def scalacOptionsFor(scalaVersion: String): Seq[String] =
ScalacOptions.defaultTokensForVersion(ScalaVersion.unsafeFromString(scalaVersion))
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no point keeping this file to me. We can just move this in TpolecatModule.scala

@davesmith00000
Copy link
Contributor Author

So I'm not sure what value mill-tpolecat really adds

Indeed, I think it's just the ability to have different configurations per environment?

What else does sbt-tpolecat provide on top of just using scalac-options in sbt? Is there some other benefit there?

@lenguyenthanh
Copy link

how bad it is to build mill's plugin with sbt?

I think of a possible way forward is archiving both sbt and mill tpolecat and move them to scalac-options.

wdyt?

@davesmith00000
Copy link
Contributor Author

how bad it is to build mill's plugin with sbt?

Couldn't say. What I do is have a shared library that contains all the core functionality (in this case the environment logic), and then have two thin projects: An sbt project publishing an sbt plugin and a Mill project publishing a Mill plugin. Their job is essentially to expose the core libs functions in ways that are idiomatic to that particular build tool.

...works well for me, but as I suggested before, I'm a bit simplistic in my approach to such things. It's likely someone could work out a cleverer set up.

@lefou
Copy link

lefou commented Oct 15, 2024

how bad it is to build mill's plugin with sbt?

Mill plugins are just ordinary Scala libraries. You should keep the Mill binary version in the artifact name, as you would do for other platforms like sbt 2.0 or Scala.JS. It gets non-trivial if you want to cross-compile for multiple Mill versions, since that isn't an out-of-the-box feature of sbt 1.x.

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.

5 participants