Skip to content

Add tlCiLintCommands key#889

Open
kubukoz wants to merge 1 commit into
typelevel:mainfrom
kubukoz:feat/tl-ci-lint-commands
Open

Add tlCiLintCommands key#889
kubukoz wants to merge 1 commit into
typelevel:mainfrom
kubukoz:feat/tl-ci-lint-commands

Conversation

@kubukoz
Copy link
Copy Markdown
Member

@kubukoz kubukoz commented May 7, 2026

Closes #888

tlCiDependencyGraphJob := true,
tlCiForkCondition := "github.event.repository.fork == false",
tlCiLintCommands := {
val headers = List("headerCheckAll").filter(_ => tlCiHeaderCheck.value)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This makes me wonder if tlCiHeaderCheck should just be deprecated in favor of it being the default here. But probably not

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm, wdym?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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.

@kubukoz kubukoz marked this pull request as ready for review May 7, 2026 23:51
Copy link
Copy Markdown
Member

@armanbilge armanbilge left a comment

Choose a reason for hiding this comment

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

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)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm, wdym?


val scalafmt = List(
"scalafmtCheckAll",
"project /",
Copy link
Copy Markdown
Member

@armanbilge armanbilge May 20, 2026

Choose a reason for hiding this comment

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

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.

Comment thread docs/customization.md
- `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.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Lint -> FastLint would be fine with me :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Public hook for appending commands to the "Check headers and formatting" lint step

2 participants