Skip to content

Conversation

greglucas
Copy link
Collaborator

This adds pre-commit to the repository. We can optionally switch over to pre-commit.ci as well which would be instead of running the linting job as a GitHub Action. Otherwise, this is mostly for myself so that it can check things automatically locally via git hooks.

There are several hooks I added that are nice to have and codespell detected some spelling errors that I fixed up here as well. I also decided to change smoot to m/s because codespell was complaining about it wanting to be smooth, this is also a more recognizable unit.

One other controversial change that I'd want to make, but will hold off on is adding ruff format check as well to auto-format the code and not have to worry about thinking about how to indent etc...

Checklist

  • Changes are fully implemented without dangling issues or TODO items
  • Deprecated/superseded code is removed or marked with deprecation warning
  • Current dependencies have been properly specified and old dependencies removed
  • New code/functionality has accompanying tests and any old tests have been updated to match any new assumptions
  • The changelog.md has been updated

@greglucas greglucas requested a review from medley56 as a code owner February 12, 2025 23:15
Copy link
Member

@medley56 medley56 left a comment

Choose a reason for hiding this comment

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

Do we also need to update documentation on setting up development environment? i.e. running pre-commit install?

@medley56
Copy link
Member

Branch needs a rebase onto or merge from main, and maybe a doc and dependency update.

Copy link
Member

@medley56 medley56 left a comment

Choose a reason for hiding this comment

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

Awesome! LGTM

@medley56 medley56 merged commit 2e454fa into lasp:main Feb 13, 2025
17 checks passed
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