Skip to content

Replace old %-formatted by f-strings#93

Merged
Fokko merged 2 commits intoapache:mainfrom
hussein-awala:switch_to_f_strings
Oct 23, 2023
Merged

Replace old %-formatted by f-strings#93
Fokko merged 2 commits intoapache:mainfrom
hussein-awala:switch_to_f_strings

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.

]
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
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