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

Remove ruff rule COM812 from ignore list #769

Closed
wants to merge 2 commits into from
Closed

Remove ruff rule COM812 from ignore list #769

wants to merge 2 commits into from

Conversation

bieniu
Copy link
Collaborator

@bieniu bieniu commented Jan 27, 2025

No description provided.

Copy link

codecov bot commented Jan 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 38.47%. Comparing base (61620d7) to head (385380c).
Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #769   +/-   ##
=======================================
  Coverage   38.47%   38.47%           
=======================================
  Files          15       15           
  Lines        1497     1497           
=======================================
  Hits          576      576           
  Misses        921      921           

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

@thecode
Copy link
Contributor

thecode commented Jan 27, 2025

I am not sure if we should enable this rule, this is the warning when it is enabled:

"warning: The following rule may cause conflicts when used with the formatter: COM812. To avoid unexpected behavior, we recommend disabling this rule, either by removing it from the select or extend-select configuration, or adding it to the ignore configuration."

The docs recommend to disable this rule if ruff-format is used:
https://docs.astral.sh/ruff/rules/missing-trailing-comma/
"We recommend against using this rule alongside the formatter. The formatter enforces consistent use of trailing commas, making the rule redundant."

We use ruff-format:

- id: ruff-format

It is also redundant once ruff-format is used as it would add trailing comma where needed.
For aiowebostv I added a comment why it is disabled so I don't get back to it:
https://github.com/home-assistant-libs/aiowebostv/blob/dba108947211b16a90259733b971ff0d0d11653d/ruff.toml#L8

@bieniu
Copy link
Collaborator Author

bieniu commented Jan 27, 2025

Oh yes, you're right, I completely forgot about this. I'm closing PR.

@bieniu bieniu closed this Jan 27, 2025
@bieniu bieniu deleted the ruff-com812 branch January 27, 2025 09:24
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