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

Replace pylint CI with ruff #10233

Merged
merged 1 commit into from
May 10, 2023
Merged

Conversation

akx
Copy link
Collaborator

@akx akx commented May 9, 2023

Describe what this pull request is trying to achieve.

This PR replaces the GitHub Actions lint action (that hasn't successfully run for a while) with a simple one based on the excellent Ruff linter.

  • For a starting point, the configuration is extremely simple and only enables the E999 (Syntax Error) lint; i.e. ruff just checks that the code is syntactically valid Python 3.10+ code. If this is fine, we (or I - I don't mind doing the work) can enable a broader set of lints in a future PR; Ruff's default configuration already reveals some possible bugs.
  • I left the broken pylint config in in the YAML file so it could be re-enabled easily later.
  • As noted in the YAML, there is purposely no cache here, and we're also purposely not using a specific version of Python to avoid having to download a new one onto the runner. This makes the total run time for the CI about 8 seconds.

Additional notes and description of your changes

I've put in similar ruff work in in Gradio; see gradio-app/gradio#3710, gradio-app/gradio#3979, gradio-app/gradio#4038

Environment this was tested in

  • OS: Windows (or well, Linux in GHA)
  • Browser: irrelevant
  • Graphics card: irrelevant

@akx akx marked this pull request as ready for review May 9, 2023 20:19
@akx akx requested a review from AUTOMATIC1111 as a code owner May 9, 2023 20:19
@AUTOMATIC1111 AUTOMATIC1111 changed the base branch from dev to ruff May 10, 2023 06:06
@AUTOMATIC1111 AUTOMATIC1111 merged commit 837d3a9 into AUTOMATIC1111:ruff May 10, 2023
@AUTOMATIC1111
Copy link
Owner

Well that's neat.

I made a ruff branch with fixes suggested by the tool. Very likely nothing is broken as a result, but that's not guaranteed. After some testing I'll merge it into dev.

@akx
Copy link
Collaborator Author

akx commented May 10, 2023

@AUTOMATIC1111 Cool 🎉 Feel free to request a review from me if you decide to PR the ruff branch into dev. (In the medium term it would probably be very nice to be able to enable e.g. I and some unused-import fixes, but those will require untangling of some import dep chains within the codebase based on a short look I took yesterday. 😄)

@akx akx deleted the fix-lint-ci branch May 10, 2023 12:49
@AUTOMATIC1111 AUTOMATIC1111 mentioned this pull request May 10, 2023
@AUTOMATIC1111
Copy link
Owner

I went through all unused-import, almost all of them are removed except for a small subset that I don't want to remove becsause there is a distinct possibility that some extensions is using them. I also enabled I, except for I001 Import block is un-sorted.

https://github.com/AUTOMATIC1111/stable-diffusion-webui/blob/ruff/pyproject.toml

PR: #10259. It's not small.

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