Skip to content

Various opinionated changes & modernization #16

Merged
adamw merged 10 commits into
softwaremill:masterfrom
chollinger93:chollinger93/modernize-scala-pre-commit
May 13, 2025
Merged

Various opinionated changes & modernization #16
adamw merged 10 commits into
softwaremill:masterfrom
chollinger93:chollinger93/modernize-scala-pre-commit

Conversation

@chollinger93
Copy link
Copy Markdown
Contributor

@chollinger93 chollinger93 commented May 8, 2025

Here are several unsolicited (and opinionated) improvements that I worked on when I wanted to use this for my own project(s). I hope you find them useful, but I'd understand if it's a bit much.

Changes

  • Fixed .pre-commit-hooks.yaml to match >= 3.20 version
  • Added an alias for scalafmt + apply
  • Fixed bug with sbt-fatal-warnings
  • Added .pre-commit-config + ran lint/fmt
  • Adds a flag for --project-dir, which is useful for monorepos that don't necessarily have a root-level build.sbt
  • Adds a flag for --no-clean, since running an sbt clean on each commit/push gets slow
  • Re-wrote the arg-parser
  • Disabled always_run, since that makes things unnecessarily slow and disables things like file filters

sbt-scalafmt-apply fixed

I noticed that sbt-scalafmt-apply didn't work:

  - repo: https://github.com/softwaremill/scala-pre-commit-hooks
    rev: v0.4.0
    hooks:
      - id: sbt-scalafmt-apply
❯ pre-commit
[ERROR] `sbt-scalafmt-apply` is not present in repository https://github.com/softwaremill/scala-pre-commit-hooks.  Typo? Perhaps it is introduced in a newer version?  Often `pre-commit autoupdate` fixes this.
❯ pre-commit autoupdate
[https://github.com/pre-commit/pre-commit-hooks] already up to date!
[https://github.com/softwaremill/scala-pre-commit-hooks] Cannot update because the update target is missing these hooks: sbt-scalafmt-apply
# commenting it out, update
❯ pre-commit autoupdate
[https://github.com/pre-commit/pre-commit-hooks] already up to date!
[https://github.com/softwaremill/scala-pre-commit-hooks] already up to date
❯ pre-commit
[ERROR] `sbt-scalafmt-apply` is not present in repository https://github.com/softwaremill/scala-pre-commit-hooks.  Typo? Perhaps it is introduced in a newer [version](https://pre-commit.com/#confining-hooks-to-run-at-certain-stages)?  Often `pre-commit autoupdate` fixes this

Why that is (might be user error), I still don't quite know, but I've fixed the fixed .pre-commit-hooks.yaml to match >= 3.20 version anyways (you get warnings otherwise) + added an alias, which seems to work fine.

Fixed bug with sbt-fatal-warnings

If you enable that on the current version:

Traceback (most recent call last):
  File "/Users/christian/.cache/pre-commit/repou3vo0cty/py_env-python3.13/bin/sbt-fatal-warnings", line 8, in <module>
    sys.exit(main())
             ~~~~^^
  File "/Users/christian/.cache/pre-commit/repou3vo0cty/py_env-python3.13/lib/python3.13/site-packages/pre_commit_hooks/sbt_fatal_warnings.py", line 18, in main
    args[ARG_ADDITIONAL_ARGS].append("-Xfatal-warnings")
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'append'

This fixes that by doing

addtl_args = args.get(ARG_ADDITIONAL_ARGS, [])

It also fixes the:

[error] Expected letter
[error] Expected symbol
...

Issue.

Added .pre-commit-config + Formatting

I've taken the liberty of adding a short .pre-commit-config.yaml, so that the code in this repo gets formatted + linted etc. It uses ruff for formatting, which is easy + fast.

I've also updated the .gitignore to a more standard format and updated the setup* files.

I've also updated the Python version to 3.9. Debian Trixie is about to code freeze with 3.13, so 3.6 seemed a bit too conservative.

New Flags

Basically, --no-clean just skips the sbt clean beforehand. I think that'll make this too slow on larger projects.

--project path just sets the subprocess cwd, so this can be used in a non-scala monorepo.

I also re-wrote the argparser a bit to clean up.

Test

  - repo: https://github.com/chollinger93/scala-pre-commit-hooks
    rev: cc82d1d5dfe52e4415a236745a40a17c0c56b656 #v0.5.0
    hooks:
      - id: sbt-fatal-warnings #arguments optional
        args: [ --scope=test:compile ]
      - id: sbt-unused-imports #includes fatal warnings, arguments optional
        args: [ --scope=test:compile ]
      - id: sbt-scalafmt
      	args: [ --no-clean ]
      - id: sbt-scalafmt-apply
      	args: [ --no-clean ]

To test the project-dir flag outside a monorepo:

python3 pre_commit_hooks/scalafmt.py --project-dir /Users/christian/workspace/bridgefour --no-clean

@adamw
Copy link
Copy Markdown
Member

adamw commented May 13, 2025

This looks good, very nicely documented! Let's merge and see if there are any bug reports :)

@adamw adamw merged commit 8c72cf7 into softwaremill:master May 13, 2025
@chollinger93
Copy link
Copy Markdown
Contributor Author

chollinger93 commented May 13, 2025

🙏 @adamw Could you create a release tag, please?

@adamw
Copy link
Copy Markdown
Member

adamw commented May 13, 2025

Ah sure, sorry, forgot :)

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.

2 participants