Add tlCiLintCommands key#889
Conversation
| tlCiDependencyGraphJob := true, | ||
| tlCiForkCondition := "github.event.repository.fork == false", | ||
| tlCiLintCommands := { | ||
| val headers = List("headerCheckAll").filter(_ => tlCiHeaderCheck.value) |
There was a problem hiding this comment.
This makes me wonder if tlCiHeaderCheck should just be deprecated in favor of it being the default here. But probably not
There was a problem hiding this comment.
I mean that instead of the current state we could have a default value of tlCiLintCommands := List("headerCheckAll", "scalafmtAll"), and get rid of tlCiHeaderCheck as a key. If someone wants to opt out of header checks, they can modify tlCiLintCommands.
armanbilge
left a comment
There was a problem hiding this comment.
Overall not against this, I think there are just some annoying gotchas to be aware of: state-changing commands and non-deterministic ordering (see http4s/sbt-http4s-org#120 (comment)).
| tlCiDependencyGraphJob := true, | ||
| tlCiForkCondition := "github.event.repository.fork == false", | ||
| tlCiLintCommands := { | ||
| val headers = List("headerCheckAll").filter(_ => tlCiHeaderCheck.value) |
|
|
||
| val scalafmt = List( | ||
| "scalafmtCheckAll", | ||
| "project /", |
There was a problem hiding this comment.
So, one thing to note, is that project / actually changes the state of the shell. So it makes a difference if commands are before or after this.
| - `tlCiDocCheck` (setting): Whether to build API docs in CI (default: `false`). | ||
| - `tlCiDependencyGraphJob` (setting): Whether to add a job to submit dependencies to GH (default: `true`). | ||
| - `tlCiForkCondition` (setting): Condition for checking on CI whether this project is a fork of another (default: `github.event.repository.fork == false`). | ||
| - `tlCiLintCommands` (setting): sbt commands run in the "Check headers and formatting" CI step. Defaults are derived from `tlCiHeaderCheck`, `tlCiScalafmtCheck`, and `tlCiJavafmtCheck`. Append your own check (e.g. `tlCiLintCommands += "myLintCheckAll"`) to fold it into the same step. Scalafix is gated separately via `tlCiScalafixCheck` and runs as its own step. |
There was a problem hiding this comment.
Bikeshedding the name ... the idea is to separate "fast" commands (formatting, headers) from "slow" commands (Scalafix, which often requires compiling the code). But I'm not sure if "lint" fully captures that, since arguably Scalafix is also a form of linting.
There was a problem hiding this comment.
Lint -> FastLint would be fine with me :)
Closes #888