Skip to content

[lint] Download linters from pytorch/pytorch #6935

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

clee2000
Copy link
Contributor

@clee2000 clee2000 commented Jul 16, 2025

I don't know if this is a good idea

When lintrunner was first introduced to test-infra, Sahan copied a bunch of linters from pytorch/pytorch, and I did the same later to update them, so there's a lot of dup code

This PR adds a script that downloads init scripts and lint scripts from a specific link, then runs them. Commit shas were selected to minimize difference between the existing scripts and the ones downloaded.

Pros:

  • less dup code

Cons:

  • confusing to track changes
  • init deps and args not kept in sync
  • other args not kept in sync

Alt ideas:

  • use lintrunner adapters pip package (but it still has the problem about init deps and args not being provided automatically, also need to figure out a way to pip install lintrunner adapters on init, also not sure if the package is up to date)

Copy link

vercel bot commented Jul 16, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Updated (UTC)
torchci ⬜️ Ignored (Inspect) Visit Preview Jul 16, 2025 4:41pm

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 16, 2025
@clee2000 clee2000 force-pushed the csl/download_linters branch from 51595fd to 31bf5de Compare July 16, 2025 16:11
@clee2000 clee2000 marked this pull request as ready for review July 16, 2025 16:30
@clee2000 clee2000 requested a review from a team July 16, 2025 16:30
@huydhn
Copy link
Contributor

huydhn commented Jul 16, 2025

Maybe we could look into https://github.com/justinchuby/lintrunner-adapters. ExecuTorch is getting common linters from there https://github.com/pytorch/executorch/blob/main/requirements-dev.txt#L12

@clee2000
Copy link
Contributor Author

Maybe we could look into https://github.com/justinchuby/lintrunner-adapters. ExecuTorch is getting common linters from there https://github.com/pytorch/executorch/blob/main/requirements-dev.txt#L12

I mention this in the PR body, do you have any thoughts on the cons I list?

@huydhn
Copy link
Contributor

huydhn commented Jul 16, 2025

I mention this in the PR body, do you have any thoughts on the cons I list?

It might be out of date like what you say, but I think we could actively maintain it going forward if needed. It will benefit more than one team.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants