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

chore: add ruff to dev deps #1957

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JP-Ellis
Copy link

@JP-Ellis JP-Ellis commented Feb 5, 2025

The pyproject.toml file contains a [tool.ruff] section, but Ruff is missing from the dev dependencies. I assume this was an oversight and this commits adds it.

I have also taken the opportunity to sort the dev (and old_deps_pins) dependencies in alphabetical order.

The `pyproject.toml` file contains a `[tool.ruff]` section, but Ruff is
missing from the dev dependencies. I assume this was an oversight and
this commits adds it.

I have also taken the opportunity to sort the dev (and `old_deps_pins`)
dependencies in alphabetical order.

Signed-off-by: JP-Ellis <[email protected]>
@yagebu
Copy link
Member

yagebu commented Feb 8, 2025

The pre-commit hooks include ruff so pre-commit sets up a separate virtual environment up for it. I don't think it needs to be added to the .venv as well (increased the size by 24MB on my machine).

@JP-Ellis
Copy link
Author

JP-Ellis commented Feb 8, 2025

The reason for including it in the virtual environment is to ensure that the IDE (VSCode in my case) finds the correct version during development, instead of waiting until I create a commit to realise I made some minor linting mistakes. It also ensures that the IDE uses the same version as the pre-commit, as opposed to a perhaps incompatible version it might find elsewhere. Overall, it just makes for a much nicer developer experience.

As to your comment about the size of the virtual environment; this is purely a dev dependency, and therefore should not impact downstream users of Fava on its own (unless they install fava[dev]...)

@JP-Ellis
Copy link
Author

I noticed there's now a merge conflict. Would you like me to update the PR to resolve that? Or is the PR a no-go?

As mentioned above, this would only be a dev dependency and therefore should not impact downstream users of the package. It will only ensure that IDEs can use the same version of ruff as in the pre-commit hook, thereby avoiding linting issues during development (as opposed to waiting for the pre-commit hook to fail).

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