Skip to content

chore: Re-add sbt-scalafmt to the sbt 2.x plugin builds#625

Closed
xerial wants to merge 1 commit into
chore/bump-sibling-sbt-201from
chore/readd-sbt-scalafmt
Closed

chore: Re-add sbt-scalafmt to the sbt 2.x plugin builds#625
xerial wants to merge 1 commit into
chore/bump-sibling-sbt-201from
chore/readd-sbt-scalafmt

Conversation

@xerial

@xerial xerial commented Jun 30, 2026

Copy link
Copy Markdown
Member

Why

While sbt-uni and sbt-uni-playwright were pinned to sbt 2.0.0-RC10, sbt-scalafmt (which needs ≥ 2.0.0-RC11) had to be left out. Now that #624 moves them to sbt 2.0.1, the formatter can be re-added — restoring the same scalafmtCheckAll enforcement the main build already has.

Stacked on #624 (base = chore/bump-sibling-sbt-201). Review/merge that first; this diff is scalafmt-only on top of it.

What

  • Add addSbtPlugin("org.scalameta" % "sbt-scalafmt" % "2.6.1") (the version the main build uses) to sbt-uni and sbt-uni-playwright metabuilds.
  • Add a local .scalafmt.conf to sbt-uni (it had none — its sources happened to already conform to the mirrored repo-root style).
  • Reformat the 7 existing sbt-uni-playwright sources to that style (they were written before the formatter was available; formatting-only, still compiles).
  • Enforce in CI: the test_sbt_plugin and test_sbt_uni_playwright jobs now run scalafmtCheckAll before scripted.

Validation

scalafmtCheckAll passes for both builds on sbt 2.0.1, and both still compile.

Follow-up: sbt-uni-crossproject (added in #623) gets the same treatment in that PR, so all three sbt 2.x builds end up consistent.

🤖 Generated with Claude Code

Now that both builds are on sbt 2.0.1, sbt-scalafmt (which needs >= 2.0.0-RC11)
can be re-added, restoring the formatter that was dropped while they were on
RC10. sbt-uni gains a local .scalafmt.conf (it had none); the existing
sbt-uni-playwright sources are reformatted to the mirrored repo-root style.

CI now enforces it: the two sub-build jobs run scalafmtCheckAll before scripted,
so formatting is checked the same way the main build's code_format job checks it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the internal Non-user facing changes label Jun 30, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request integrates the sbt-scalafmt plugin and reformats the codebase according to the newly added .scalafmt.conf configuration. The review feedback identifies a potential resource leak in PlaywrightEngine.scala where an exception thrown during streams.close() in the finally block could prevent subsequent cleanups of the browser session and temporary files.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +98 to +99
if streams != null then
streams.close()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If streams.close() throws an exception, the subsequent critical cleanups (session.close() and materializer.close()) will be skipped. This can lead to leaked browser processes (Playwright) and leftover temporary files. Wrapping streams.close() in a try-catch block ensures that browser shutdown and file cleanup always execute.

      if streams != null then
        try streams.close()
        catch case NonFatal(_) => ()

@xerial xerial deleted the branch chore/bump-sibling-sbt-201 June 30, 2026 19:44
@xerial xerial closed this Jun 30, 2026
xerial added a commit that referenced this pull request Jun 30, 2026
## Why

While `sbt-uni` and `sbt-uni-playwright` were pinned to sbt
`2.0.0-RC10`, `sbt-scalafmt` (needs ≥ `2.0.0-RC11`) had to be left out.
Now that they're on **sbt 2.0.1** (#624, merged), the formatter can be
re-added — restoring the same `scalafmtCheckAll` enforcement the main
build already has.

(Supersedes #625, which GitHub auto-closed when its stacked base branch
was deleted on merge. Rebased onto main; diff is scalafmt-only.)

## What

- Add `addSbtPlugin("org.scalameta" % "sbt-scalafmt" % "2.6.1")` (the
main build's version) to `sbt-uni` and `sbt-uni-playwright` metabuilds.
- Add a local `.scalafmt.conf` to `sbt-uni` (it had none — its sources
already conform).
- Reformat the 7 existing `sbt-uni-playwright` sources (formatting-only;
still compiles).
- **Enforce in CI**: `test_sbt_plugin` and `test_sbt_uni_playwright` now
run `scalafmtCheckAll` before `scripted`.

## Validation

`scalafmtCheckAll` passes for both builds on sbt 2.0.1, and both still
`compile`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Non-user facing changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant