-
Notifications
You must be signed in to change notification settings - Fork 49
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
Hello, sbt-typelevel #12
Conversation
Alright, definitely not perfect, but overall seems to be functional :) addSbtPlugin("com.armanbilge" % "sbt-typelevel-ci-release" % "0.4-ddf26e7")
enablePlugins(TypelevelCiReleasePlugin) https://index.scala-lang.org/armanbilge/sbt-typelevel/ Notable features
Full list of pluginsaddSbtPlugin("com.armanbilge" % "sbt-typelevel-no-publish" % "0.4-ddf26e7")
addSbtPlugin("com.armanbilge" % "sbt-typelevel-settings" % "0.4-ddf26e7")
addSbtPlugin("com.armanbilge" % "sbt-typelevel-versioning" % "0.4-ddf26e7")
addSbtPlugin("com.armanbilge" % "sbt-typelevel-mima" % "0.4-ddf26e7")
addSbtPlugin("com.armanbilge" % "sbt-typelevel-sonatype" % "0.4-ddf26e7")
addSbtPlugin("com.armanbilge" % "sbt-typelevel-sonatype-ci-release" % "0.4-ddf26e7")
addSbtPlugin("com.armanbilge" % "sbt-typelevel-ci-signing" % "0.4-ddf26e7")
addSbtPlugin("com.armanbilge" % "sbt-typelevel-ci" % "0.4-ddf26e7")
addSbtPlugin("com.armanbilge" % "sbt-typelevel" % "0.4-ddf26e7")
addSbtPlugin("com.armanbilge" % "sbt-typelevel-ci-release" % "0.4-ddf26e7") |
|
||
override def buildSettings = | ||
addCommandAlias( | ||
"ci", |
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.
Thought exercise: how would we make this open to extension for projects that run additional checks? This has frustrated me in other plugins that use aliases.
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.
Oh, replaceCommandAlias
?
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.
Yep, got that here:
sbt-typelevel/kernel/src/main/scala/org/typelevel/sbt/TypelevelKernelPlugin.scala
Lines 11 to 15 in f277ca9
object autoImport { | |
lazy val tlIsScala3 = settingKey[Boolean]("True if building with Scala 3") | |
def replaceCommandAlias(name: String, contents: String): Seq[Setting[State => State]] = | |
Seq(GlobalScope / onLoad ~= { (f: State => State) => | |
f andThen { s: State => |
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.
So much else is structured, and we manipulate aliases as semi-colon delimited strings. But I guess we inherit that from SBT's model. This is probably the best we can do.
import GitHubActionsPlugin.autoImport._ | ||
|
||
override def globalSettings = Seq( | ||
tlFatalWarningsInCi := true |
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.
I have never been particularly fond of this setting varying from local development.
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.
True. But also, fatal warnings make development/experimentation extremely annoying.
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.
I generally solve that by adjusting my scalacOptions in session while in that mode. That way it's consistent with CI by default, and I can opt out while hacking. But I won't die on this hill if other people like it.
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.
Interesting. OTOH I generally solve this problem by actually paying attention to the warnings indicated in the IDE 😉
One compromise is providing a prePR
command that emulates a CI compile.
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.
I like the compromise, but I thought that was the intent of the ci
alias?
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.
I always thought of ci
as what you want CI to do—run the entire test suite, run MiMa, generate docs, etc. etc. Meanwhile, prePR
are "lighter-weight" checks you can do locally, e.g. formatting, compiling with fatal warnings, etc.
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.
Hmm, this is trickier to implement than I thought.
addCommandAlias(
"prePR",
"; root/clean; set tlFatalWarnings := tlFatalWarningsInCi.value; +root/Test/compile")
The problem is, how do you reset tlFatalWarnings
to the old setting after prePR
? Not sure if this is possible, or my sbt-fu isn't good enough anyway. Someone smarter can chime in, otherwise we can shunt this for now.
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.
It could be done with a custom task, but it may not be worth the trouble.
|
||
object autoImport { | ||
lazy val tlIsScala3 = settingKey[Boolean]("True if building with Scala 3") | ||
def replaceCommandAlias(name: String, contents: String): Seq[Setting[State => State]] = |
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.
Does this deserve a tl
prefix since it's auto-imported?
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.
auto-imports definitely need a prefix. The similar isDotty
setting in sbt-spiewak is the reason http4s is not in the Scala 3 Community Build:
versioning/src/main/scala/org/typelevel/sbt/TypelevelVersioningPlugin.scala
Outdated
Show resolved
Hide resolved
Co-authored-by: Ross A. Baker <[email protected]>
lazy val tlUntaggedAreSnapshots = | ||
settingKey[Boolean]( | ||
"If true, an untagged commit is given a snapshot version, e.g. 0.4-00218f9-SNAPSHOT. If false, it is given a release version, e.g. 0.4-00218f9. (default: true)") |
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.
@rossabaker wdyt?
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.
I like it.
Please let me know what else is needed here so we can land this. I know there's still a lot to iterate on, but at least for me it'll be much easier to keep track of in issues and separate PRs rather than trying to track all the comments in this one. Thanks! After this merges I'll open follow-up issues for all the unresolved comments. |
Nice, was able to publish a snapshot under |
build.sbt
Outdated
|
||
enablePlugins(TypelevelCiReleasePlugin) | ||
ThisBuild / tlCiReleaseSnapshots := true | ||
ThisBuild / tlCiReleaseBranches := Seq("series/armanbilge") |
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.
I'll change this before we merge, but for now lets me publish from my branch.
ThisBuild / tlCiReleaseBranches := Seq("series/armanbilge") | |
ThisBuild / tlCiReleaseBranches := Seq("main") |
Darn, I'll have to rollback the |
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.
I don't think there are any showstoppers here. I'll give the snapshot a try on a lower-profile project.
Executes #11.
Early days still, but it's solid enough to self-host and publish snapshots:
https://s01.oss.sonatype.org/content/repositories/snapshots/com/armanbilge/sbt-typelevel-ci-release_2.12_1.0/0.4-a57db49-SNAPSHOT/
If you feel empowered, please give it a try.
https://index.scala-lang.org/armanbilge/sbt-typelevel/
Highlights:
Support for the former is not quite ready yet,and haven't tested the latter. Confirmed to work for both!Any and all feedback much appreciated, although it would be cool to get this merged as-is and the feedback can come in the form of issues or ideally PRs. I don't claim to have any real knowledge of what I was doing here, just tried my best 🙃
Update: published a stable hash release to maven.
Last step: getting keys with passphrases to work.Done.