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

build: format codebase imports using ruff linter #1028

Merged
merged 1 commit into from
Feb 13, 2025

Conversation

leseb
Copy link
Contributor

@leseb leseb commented Feb 10, 2025

What does this PR do?

  • Configured ruff linter to automatically fix import sorting issues.
  • Set --exit-non-zero-on-fix to ensure non-zero exit code when fixes are applied.
  • Enabled the 'I' selection to focus on import-related linting rules.
  • Ran the linter, and formatted all codebase imports accordingly.
  • Removed the black dep from the "dev" group since we use ruff

Signed-off-by: Sébastien Han [email protected]

Test Plan

[Describe the tests you ran to verify your changes with result summaries. Provide clear instructions so the plan can be easily re-executed.]

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Feb 10, 2025
- Configured ruff linter to automatically fix import sorting issues.
- Set --exit-non-zero-on-fix to ensure non-zero exit code when fixes are applied.
- Enabled the 'I' selection to focus on import-related linting rules.
- Ran the linter, and formatted all codebase imports accordingly.
- Removed the black dep from the "dev" group since we use ruff

Signed-off-by: Sébastien Han <[email protected]>
- id: ruff
args: [
--fix,
--exit-non-zero-on-fix
--exit-non-zero-on-fix,
--select, I,
Copy link
Contributor

Choose a reason for hiding this comment

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

should we also add F ( for unused imports and variables ) ?
cc @ashwinb

Copy link
Contributor Author

Choose a reason for hiding this comment

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

F is now failing with a lot of errors so perhaps we could add this in a followup PR? WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think unused imports would be really valuable. Does F include both imports and variables?

@leseb leseb requested a review from hardikjshah February 13, 2025 16:35
Copy link
Contributor

@ashwinb ashwinb left a comment

Choose a reason for hiding this comment

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

oh thank you thank you!

@ashwinb ashwinb merged commit e4a1579 into meta-llama:main Feb 13, 2025
3 checks passed
franciscojavierarceo pushed a commit to franciscojavierarceo/llama-stack that referenced this pull request Feb 14, 2025
# What does this PR do?

- Configured ruff linter to automatically fix import sorting issues.
- Set --exit-non-zero-on-fix to ensure non-zero exit code when fixes are
applied.
- Enabled the 'I' selection to focus on import-related linting rules.
- Ran the linter, and formatted all codebase imports accordingly.
- Removed the black dep from the "dev" group since we use ruff

Signed-off-by: Sébastien Han <[email protected]>

[//]: # (If resolving an issue, uncomment and update the line below)
[//]: # (Closes #[issue-number])

## Test Plan
[Describe the tests you ran to verify your changes with result
summaries. *Provide clear instructions so the plan can be easily
re-executed.*]

[//]: # (## Documentation)
[//]: # (- [ ] Added a Changelog entry if the change is significant)

Signed-off-by: Sébastien Han <[email protected]>
@leseb leseb deleted the rm-black branch February 14, 2025 07:58
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 Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants