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

[PROPOSAL+WIP] implement a ruff analyze live #15178

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

Conversation

purajit
Copy link
Contributor

@purajit purajit commented Dec 29, 2024

MVP implementation of #15198.

This is definitely not close to the final state of the PR (docs, tests, potential optimizations, Rust idiomaticity,
code refactoring at the very least); I just wanted to see I could get a reasonable implementation of this working.
I've been using this on our largest repo to test, and it's working really well. Feature-wise, I'd probably want to
add more configuration options (what to monitor, what to ignore), and also allow for xargs's replstr format to
specify where in the command the file names should be injected (currently just doing it at the end, which should
work in most cases, even with named args).

Copy link
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@MichaReiser MichaReiser added the needs-decision Awaiting a decision from a maintainer label Dec 30, 2024
@MichaReiser
Copy link
Member

Thanks for opening this PR and suggesting this idea. It's an interesting use case, but it's not fully clear to me how it fits into ruff analyze and ruff overall and if there are ways we can avoid unnecessary code duplication (Ruff now has multiple entry points that support watch).

Unfortunately, I don't have the time right now to think this through but maybe someone else on the team is excited about it and has time to help design a feature that solves the same needs and fits nicely into Ruff.

@purajit
Copy link
Contributor Author

purajit commented Dec 30, 2024

Thanks for the response!

I guess the difference is whether a tool like this should be built on top of ruff or directly powered
by ruff. I leaned towards the latter because it is directly based on generating the dependency graph
repeatedly and keeping the view updated incrementally in-memory; keeping it in analyze would
also allow this feature to be powered by anything else that gets added to analyze, and other ruff
internals. Of course, the same could be accomplished by a tool that uses ruff and reads the graph
output, but this is far more powerful, and I think it's a powerful canonical feature to have - honestly
one of the biggest benefits of having a dependency graph.

Another argument is that Pants currently has a --loop which this would be able to replace for
people coming from that world.

I do understand that this pushes the scope of ruff, I definitely expected some hesitation and discussion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-decision Awaiting a decision from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants