Skip to content

Forbid comments in f strings#3407

Merged
sobolevn merged 35 commits intowemake-services:masterfrom
LordGvozd:forbid-comments-in-f-strings
May 12, 2025
Merged

Forbid comments in f strings#3407
sobolevn merged 35 commits intowemake-services:masterfrom
LordGvozd:forbid-comments-in-f-strings

Conversation

@LordGvozd
Copy link
Copy Markdown
Contributor

I have made things!

Checklist

  • I have double checked that there are no unrelated changes in this pull request (old patches, accidental config files, etc)
  • I have created at least one test case for the changes I have made
  • I have updated the documentation for the changes I have made
  • I have added my changes to the CHANGELOG.md

Related issues

Closes #3404

@codecov
Copy link
Copy Markdown

codecov Bot commented May 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (1d95728) to head (b148d57).
⚠️ Report is 78 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #3407   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          362       362           
  Lines        11933     11945   +12     
  Branches       816       817    +1     
=========================================
+ Hits         11933     11945   +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Please, make sure that this feature is only checked on version that support it. And add the same version info to violation docs.

Comment thread wemake_python_styleguide/violations/best_practices.py Outdated
Comment thread wemake_python_styleguide/violations/best_practices.py Outdated
Comment thread wemake_python_styleguide/visitors/tokenize/comments.py Outdated
Copy link
Copy Markdown
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Almost there!

Comment thread wemake_python_styleguide/violations/best_practices.py Outdated
"""


multiline_fstring_with_comment = """
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please, also test strings with ' and """

Comment thread wemake_python_styleguide/visitors/tokenize/comments.py Outdated
@LordGvozd
Copy link
Copy Markdown
Contributor Author

Thanks for the review! But I don't understand why CI is breaking? Am I missing something?

@sobolevn
Copy link
Copy Markdown
Member

sobolevn commented May 6, 2025

Coverage on older python versions is not properly configured.

Copy link
Copy Markdown
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks! 🎉

@sobolevn
Copy link
Copy Markdown
Member

sobolevn commented May 8, 2025

The only thing left to do is to fix conditional coverage :)

@sobolevn
Copy link
Copy Markdown
Member

Use 312.py suffix to exclude file from main coverage :)

@LordGvozd
Copy link
Copy Markdown
Contributor Author

@sobolevn Thanks!

Copy link
Copy Markdown
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Awesome, we are almost there!


Example::

# Correct:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why did you remove this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Accidentally deleted while fixing conflicts )

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is not resolved :)

Comment thread wemake_python_styleguide/visitors/tokenize/comments.py
}'world"
"""

fstring_with_comment_in_docstring = '''
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please, add wrong examples for fr and rf cases with ' and " and ''' and """


Example::

# Correct:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is not resolved :)

Copy link
Copy Markdown
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

LGTM :)

@sobolevn
Copy link
Copy Markdown
Member

Thanks a lot, really helpul!

@sobolevn sobolevn merged commit 5e53250 into wemake-services:master May 12, 2025
7 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.

Forbid comments in f and t string

2 participants