Skip to content

ci: add ruff format and ruff check - #10

Open
kkysen wants to merge 6 commits into
mainfrom
kkysen/ci-ruff
Open

ci: add ruff format and ruff check#10
kkysen wants to merge 6 commits into
mainfrom
kkysen/ci-ruff

Conversation

@kkysen

@kkysen kkysen commented Nov 17, 2025

Copy link
Copy Markdown
Contributor

This adds ruff format --check (formatting) and ruff check (linting) to CI, and also runs them on the codebase first to fix errors. Almost all of these are automatically fixed, except for a couple of unused variables in ad68c42 that I fixed manually by prefixing them with _.

@kkysen
kkysen requested a review from spernsteiner November 17, 2025 17:20

@spernsteiner spernsteiner 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.

Please back out the ruff format parts of this. The ruff check parts seem good (most of the variables it flagged as unused can actually just be removed)

Comment thread crisp/__main__.py Outdated
Comment thread crisp/llm.py Outdated
Comment thread crisp/sandbox/sudo.py Outdated

@kkysen kkysen left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Please back out the ruff format parts of this.

Why are you opposed to standardized formatting? This follows the standard formatting PEPs and makes it much easier to work as a team without conflicts about little formatting differences, and to be able to read idiomatic Python that people are used to reading.

Base automatically changed from kkysen/ci-uv to main November 17, 2025 22:23
@spernsteiner

Copy link
Copy Markdown
Contributor

Why are you opposed to standardized formatting?

Following the major points of a common style guide is good, but trying to enforce every last minor point (as rustfmt does, and apparently ruff too) IME generally makes code less readable, not more. Whitespace is an important tool for conveying emphasis and structure. Automated tools have no idea what parts of the code are important and often expand and compress things in ways that make it harder to pick up on the overall shape of the code.

makes it much easier to work as a team without conflicts about little formatting differences

I've never seen any notable disagreement over formatting on my other main project, which involves a large Haskell codebase with dozens of contributors over many years (and no official style guide, much less an autoformatter). The code isn't perfectly consistent, but everyone follows some kind of reasonable style (even if not everyone follows precisely the same one), so the code is all plenty readable.

@kkysen kkysen left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Why are you opposed to standardized formatting?

Following the major points of a common style guide is good, but trying to enforce every last minor point (as rustfmt does, and apparently ruff too) IME generally makes code less readable, not more. Whitespace is an important tool for conveying emphasis and structure. Automated tools have no idea what parts of the code are important and often expand and compress things in ways that make it harder to pick up on the overall shape of the code.

makes it much easier to work as a team without conflicts about little formatting differences

I've never seen any notable disagreement over formatting on my other main project, which involves a large Haskell codebase with dozens of contributors over many years (and no official style guide, much less an autoformatter). The code isn't perfectly consistent, but everyone follows some kind of reasonable style (even if not everyone follows precisely the same one), so the code is all plenty readable.

You're able to turn off automated formatted in the few places where it's important to format it differently, but there's a reason why most collaborative projects use automated formatting. I'm already finding it much more difficult to make changes without being able to format anything lest I create huge diffs, and we're bound to get significant git conflicts as different people format things differently, and this has repeatedly and consistently happened in every project I've worked on that didn't auto-format their code.

The Python code currently here is very un-idiomatic in a lot of places, and that makes it significantly harder to read at times. Using standard formatting that programmers are used to reading would help a lot with this. And then when I make changes, I now how to manually format them instead of auto-formatting it, and that means I have to spend less time on making meaningful semantic changes. So I'll ask to please use standard formatting so that multiple people can more seamlessly contribute.

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