Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add treefmt and re-format codebase #237

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jaen
Copy link
Contributor

@jaen jaen commented May 25, 2024

Sets up treefmt for:

  • nix with nixfmt-rfc-style,
  • Python with ruff (supposedly a flake8 drop-in replacement) and mypy,
  • shell scripts with shellcheck and shellfmt,
  • GitHub actions withactionlint.

I have followed excludes from https://github.com/nix-community/robotnix/blob/master/scripts/check.sh and fixed any issues (minor type/lint issues in Python). Next step would be probably to re-enable checking this in CI.

This is based on top of #236, so it has to go in first.

@jaen jaen force-pushed the format-codebase branch 4 times, most recently from 0106dad to 79cd7a5 Compare May 26, 2024 13:04
@jaen
Copy link
Contributor Author

jaen commented May 26, 2024

Hm, maybe I should add treefmt-nix (and any future development dependencies) using this pattern – https://figsoda.github.io/posts/2023/developing-nix-libraries-with-subflakes/? I haven't tried it before, but it's probably a good idea to try and avoid propagating treefmt to users?

EDIT: nevermind, forgot how error-prone that is — I thin it's ultimately better to suffer a few supwrfluous dependencies than weird corner–cases of NixCpp.

@jaen jaen marked this pull request as draft May 26, 2024 14:21
@jaen
Copy link
Contributor Author

jaen commented May 27, 2024

I think this is ultimately ready for review, but will be easier too look at after #236 is merged, as they share commits and I don't really see way to stack PRs against forks in GH, like you can in a single repo.

@jaen jaen marked this pull request as ready for review May 27, 2024 19:44
@jaen jaen force-pushed the format-codebase branch 2 times, most recently from 251a0ea to 8d1fd2b Compare June 7, 2024 11:01
@jaen jaen mentioned this pull request Jun 7, 2024
@jaen
Copy link
Contributor Author

jaen commented Jun 8, 2024

Back to a draft for now due to tooling discussion in #239 (comment)

@jaen jaen marked this pull request as draft June 8, 2024 12:28
@jaen
Copy link
Contributor Author

jaen commented Jun 15, 2024

Treefmt now resolved those two issues:
numtide/treefmt#316
numtide/treefmt#317
This means it should be good to merge again.

@jaen jaen marked this pull request as ready for review June 15, 2024 19:30
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.

1 participant