Skip to content

Add basic spin config and linting commands#167226

Closed
zklaus wants to merge 10 commits intogh/zklaus/19/basefrom
gh/zklaus/19/head
Closed

Add basic spin config and linting commands#167226
zklaus wants to merge 10 commits intogh/zklaus/19/basefrom
gh/zklaus/19/head

Conversation

@zklaus
Copy link
Collaborator

@zklaus zklaus commented Nov 6, 2025

This PR adds a basic spin configuration to allow for linting. It is designed as a drop-in replacement for the current Makefile based solution, i.e. it sets up and updates lintrunner based on the hashes of certain configuration files.

Lintrunner is called via Uv's uvx command, separating its environment from the general development environment in an effort to reduce instances of competing requirements breaking environments.

Stack from ghstack (oldest at bottom):

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Nov 6, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/167226

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (1 Unrelated Failure)

As of commit 0c1fbb7 with merge base 2c846bb (image):

UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

Thanks!
Is "spin lint" all green for you locally?

[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
Copy link
Contributor

@atalman atalman left a comment

Choose a reason for hiding this comment

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

lgtm. Thank you.

[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
@zklaus
Copy link
Collaborator Author

zklaus commented Nov 13, 2025

@albanD, I think I've addressed all comments and the tests are green. What do you think?

Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

Sounds good!

As a follow up question. Is there a way to have a version where I can pass, as a user, any flag I want to lintrunner?
For example, doing some lint bringup work, I want to run lintrunner --take RUFF --all-files.
Doesn't have to be in this PR, but can be a followup.

}


ALL_LINTERS = VERY_FAST_LINTERS | FAST_LINTERS | SLOW_LINTERS
Copy link
Collaborator

Choose a reason for hiding this comment

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

What will happen to new lints as they're being added?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They need to be categorized. As it is now, an unknown linter will abort the operation. We could change that to a warning while still moving ahead otherwise.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm happy with an error as long as the error message is very clear on what the dev seeing it should do (I guess timing it and putting it in the right category).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The warning (in yellow) for an uncategorized linter TESTI is Unknown linters found; please add them to the correct category in .spin/cmds.py: TESTI. All other linters will be run, but not this one.

@zklaus
Copy link
Collaborator Author

zklaus commented Nov 14, 2025

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Nov 14, 2025
@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

Khanaksahu pushed a commit to Khanaksahu/pytorch that referenced this pull request Nov 17, 2025
Silv3S pushed a commit to Silv3S/pytorch that referenced this pull request Nov 18, 2025
This PR adds a basic spin configuration to allow for linting. It is designed as a drop-in replacement for the current Makefile based solution, i.e. it sets up and updates lintrunner based on the hashes of certain configuration files.

Lintrunner is called via Uv's `uvx` command, separating its environment from the general development environment in an effort to reduce instances of competing requirements breaking environments.

Pull Request resolved: pytorch#167226
Approved by: https://github.com/atalman, https://github.com/albanD
pytorchmergebot pushed a commit that referenced this pull request Nov 20, 2025
This adds basic documentation of the linting features for Spin added in #167226 to the CONTRIBUTING.md document.

Pull Request resolved: #167227
Approved by: https://github.com/atalman, https://github.com/albanD
JacobSzwejbka pushed a commit that referenced this pull request Dec 8, 2025
This adds basic documentation of the linting features for Spin added in #167226 to the CONTRIBUTING.md document.

Pull Request resolved: #167227
Approved by: https://github.com/atalman, https://github.com/albanD
@github-actions github-actions bot deleted the gh/zklaus/19/head branch December 15, 2025 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk Trigger trunk jobs on your pull request Merged open source release notes: devx topic: devs Developer feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants