Skip to content

[pylint] make fix unsafe if delete comments (PLR1730)#17459

Merged
ntBre merged 3 commits intoastral-sh:mainfrom
VascoSch92:issue-17311
Apr 18, 2025
Merged

[pylint] make fix unsafe if delete comments (PLR1730)#17459
ntBre merged 3 commits intoastral-sh:mainfrom
VascoSch92:issue-17311

Conversation

@VascoSch92
Copy link
Contributor

The PR dresses issue #17311

@github-actions
Copy link
Contributor

github-actions bot commented Apr 18, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@ntBre ntBre linked an issue Apr 18, 2025 that may be closed by this pull request
Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

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

Thanks! This looks good, just two nits (and you may need to reformat the example code block to pass CI, if we decide to keep it)

VascoSch92 and others added 2 commits April 18, 2025 17:25
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
@VascoSch92
Copy link
Contributor Author

VascoSch92 commented Apr 18, 2025

Hey @ntBre
I addressed your feedbacks. I hope it is what you wanted.

For the example: I took inspiration from here
But I can delete it. No problem ;-)

PS: the CI is failing because of some red knot stuff. I don't know how this is related to my changes

@ntBre
Copy link
Contributor

ntBre commented Apr 18, 2025

Hey @ntBre I addressed your feedbacks. I hope it is what you wanted.

Thanks!

For the example: I took inspiration from here But I can delete it. No problem ;-)

That rule applies to stub files, and the ## Example section also uses pyi. This rule is for non-stub files, so we should use py or python in the code blocks.

PS: the CI is failing because of some red knot stuff. I don't know how this is related to my changes

Yeah no worries, main was actually failing for a little while, and your PR must have picked that up. I'll close and re-open the PR to retrigger CI because I think this is good to land otherwise.

@ntBre ntBre closed this Apr 18, 2025
@ntBre ntBre reopened this Apr 18, 2025
@ntBre ntBre added the fixes Related to suggested fixes for violations label Apr 18, 2025
@ntBre ntBre merged commit 5fec103 into astral-sh:main Apr 18, 2025
39 of 43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fixes Related to suggested fixes for violations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PLR1730 "safe" fix may delete comments

2 participants

Comments