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 old %-formatted by f-strings #93

Merged
merged 2 commits into from
Oct 23, 2023

Conversation

hussein-awala
Copy link
Member

Replace old %-formatted by f-strings and add flynt hook to pre-commit to auto-detect and fix them.

@@ -70,3 +70,11 @@ repos:
]
additional_dependencies:
- tomli==2.0.1
- repo: https://github.com/ikamensh/flynt
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of adding a new plugin, can you enable the check in Ruff? https://docs.astral.sh/ruff/rules/#flynt-fly

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I will check it.

Copy link
Member Author

Choose a reason for hiding this comment

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

The implementation of Flynt in ruff is still in progress (astral-sh/ruff#2102), the current rule doesn't detect all the usages of the old styles, and I cannot provide the argument line-length to deal with long lines. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I was unaware of that. I ran it locally, and it didn't pick up the violations that are caught here.

Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

Great suggestion @hussein-awala 🙌

@Fokko Fokko changed the title Replace old %-formatted by f-strings Replace old %-formatted by f-strings Oct 23, 2023
@Fokko Fokko merged commit 4616d03 into apache:main Oct 23, 2023
6 checks passed
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