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.

Sign up for GitHub

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

Jump to bottom

refactor(tooling): Switch to ruff-format for formatting. #1169

Merged
merged 2 commits into from
Jan 28, 2025
Merged

refactor(tooling): Switch to ruff-format for formatting. #1169

merged 2 commits into from
Jan 28, 2025

Conversation

elenakrittik
Copy link
Contributor

@elenakrittik elenakrittik commented Mar 19, 2024 •
edited
Loading

Summary

Replaces black with ruff-format.

Depends on #1168

Checklist

  • If code changes were made, then they have been tested
    • I have updated the documentation to reflect the changes
    • I have formatted the code properly by running pdm lint
    • I have type-checked the code by running pdm pyright
  • This PR fixes an issue
  • This PR adds something new (e.g. new method or parameters)
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, ...)

@shiftinv shiftinv added t: meta Changes to the project itself (CI, configs, etc.) t: refactor/typing/lint Refactors, typing changes and/or linting changes labels Mar 19, 2024
@elenakrittik elenakrittik force-pushed the refactor/replace-black branch from 3cfc636 to e47a5a2 Compare March 19, 2024 16:35
@elenakrittik elenakrittik marked this pull request as ready for review March 19, 2024 16:36
@elenakrittik elenakrittik force-pushed the refactor/replace-black branch from e47a5a2 to fbd43e1 Compare March 19, 2024 18:44
@elenakrittik elenakrittik force-pushed the refactor/replace-black branch from fbd43e1 to fe1372d Compare March 29, 2024 12:30
@elenakrittik elenakrittik force-pushed the refactor/replace-black branch 3 times, most recently from aa96ddf to 62778ff Compare May 14, 2024 13:05
@elenakrittik
Copy link
Contributor Author

Ready for consideration!

shiftinv
shiftinv requested changes May 19, 2024
View reviewed changes
Copy link
Member

@shiftinv shiftinv left a comment

Choose a reason for hiding this comment

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

I think I voiced my concerns about all-in-one tooling on discord before; to summarize, not being able to update linter and formatter separately anymore isn't ideal.
If one wants to update ruff to fix a linter bug, but doesn't want the formatting changes a newer version would potentially bring, you're pretty much out of luck.

Still, I'm willing to give this a try, especially given the pretty substantial performance difference. :>
There's potential for lots of fun merge conflicts in other PRs here, so I'd prefer only merging this after the next release - hope that's understandable

disnake/ext/commands/params.py Outdated Show resolved Hide resolved
disnake/types/activity.py Outdated Show resolved Hide resolved
@elenakrittik
Copy link
Contributor Author

I think I voiced my concerns about all-in-one tooling on discord before; to summarize, not being able to update linter and formatter separately anymore isn't ideal.

Don't see much of an issue with that, but given that

I'd prefer only merging this after the next release

We'll have a chance to test it 😄

Enegg
Enegg suggested changes May 19, 2024
View reviewed changes
Copy link
Contributor

@Enegg Enegg left a comment •
edited
Loading

Choose a reason for hiding this comment

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

165ab84 Undo the formatting changes too

@elenakrittik
Copy link
Contributor Author

elenakrittik commented May 19, 2024 •
edited
Loading

165ab84 Undo the formatting changes too

Sorry, why?

@Enegg
Copy link
Contributor

Enegg commented May 19, 2024

Excuse me, why?

What's the point of fmt: off if the thing it was guarding from being formatted, has been formatted?
https://github.com/DisnakeDev/disnake/pull/1169/files#diff-6377fad5f75948c1eaa880c932839dd3f47b01c21056433ada31632cb518a665R480-R499

@shiftinv shiftinv added this to the disnake v2.11 milestone May 21, 2024
@elenakrittik
Copy link
Contributor Author

Ruff v0.5 has just been released, so technically this can move forward. I'll try and find some time to update this

@shiftinv
Copy link
Member

Would it be possible for you to update this, now that the next version has been released? The library already uses ruff v0.8.0 now, and it appears that there haven't been any format changes between v0.3.4 and v0.8.0, plus only a couple minor string format changes from v0.8.0 to v0.9.3/latest. 👀

@elenakrittik elenakrittik force-pushed the refactor/replace-black branch from 165ab84 to 9286d74 Compare January 27, 2025 18:40
@elenakrittik elenakrittik force-pushed the refactor/replace-black branch from 9286d74 to 110f4f1 Compare January 27, 2025 19:01
@elenakrittik
Copy link
Contributor Author

Should be good now!

@elenakrittik
Copy link
Contributor Author

elenakrittik commented Jan 28, 2025 •
edited
Loading

Fixes both issues! That was a silly oversight

shiftinv
shiftinv approved these changes Jan 28, 2025
View reviewed changes
Copy link
Member

@shiftinv shiftinv 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!

@shiftinv shiftinv merged commit 38ba687 into DisnakeDev:master Jan 28, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Enegg Enegg Enegg requested changes

@shiftinv shiftinv shiftinv approved these changes

Assignees
No one assigned
Labels
skip news t: meta Changes to the project itself (CI, configs, etc.) t: refactor/typing/lint Refactors, typing changes and/or linting changes
Projects
disnake
Status: Done
Milestone
disnake v2.11
Development

Successfully merging this pull request may close these issues.
3 participants
@elenakrittik @Enegg @shiftinv