pFad - Phone/Frame/Anonymizer/Declutterfier! Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

URL: http://github.com/libgit2/pygit2/pull/1350

bassets.com/assets/primer-primitives-7f694b60439d06c0.css" /> typecheck-part-1 by DinhHuy2010 · Pull Request #1350 · libgit2/pygit2 · GitHub
Skip to content

typecheck-part-1#1350

Open
DinhHuy2010 wants to merge 13 commits intolibgit2:masterfrom
DinhHuy2010:typecheck-part-1
Open

typecheck-part-1#1350
DinhHuy2010 wants to merge 13 commits intolibgit2:masterfrom
DinhHuy2010:typecheck-part-1

Conversation

@DinhHuy2010
Copy link
Copy Markdown

Split from #1332

Comment thread pyproject.toml Outdated
Comment on lines +31 to +44
lint.select = [
"E",
"W",
"F",
"I",
"B",
"C4",
"ARG",
"SIM",
"PTH",
"PL",
"TID",
]
lint.ignore = [
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

How come you're not using this instead, for select and ignore?

[tool.ruff.lint]

As seen (and suggested one might say) in ruff's docs.

Comment thread pyproject.toml Outdated

[tool.ruff]
target-version = "py310" # oldest supported Python version
fix = true
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shouldn't fixes be opt-in, instead of opt-out? 🤔 Would you mind elaborating on your reasoning?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

oops, that should be opt-in, sorry.

Comment thread pyproject.toml Outdated
Comment on lines +45 to +56
"W291", # Trailing whitespace
"E501", # Line too long
"W293", # Blank line contains whitespace
"PLR0912", # Too many branches
"PLR2004", # Magic values
"PLR0915", # Too many statements
"PLW0603", # Global statement
"PLR0913", # Too many arguments
"B010", # setattr
"F401", # unused imports
"ARG002", # unused arguments
"SIM105", # try-except-pass
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Are you ignoring them because there are many fixes to be made?

If so, the suggested approach is not to ignore them at the configuration level, but run ruff with --add-noqa, as seen here.

When enabling a new rule on an existing codebase, you may want to ignore all existing violations of that rule and instead focus on enforcing it going forward.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ah, never know that kind of thing, thanks!

Comment thread pyproject.toml
[tool.ruff.format]
quote-style = "single"

[tool.pyright]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You are adding a pyright configuration but pyright is not in the repo in any capacity.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

will add pyright soon in another PR

@jdavid
Copy link
Copy Markdown
Member

jdavid commented Feb 25, 2025

Thanks @DinhHuy2010 for the contribution. And thanks @Tsafaras for the review, I agree with the comments.

Regarding select/ignore, it should be more specific. I've pushed a commit where just the F401, F403 and F405 rules are ignored, and only in the concerned files pygit2/__init__.py and pygit2/ffi.py

Now ruff check pass, so I think this PR is not relevant at this time (maybe some changes here are related to PR #1332)

@DinhHuy2010 Could you open other PRs with work from PR #1332 ?

@DinhHuy2010
Copy link
Copy Markdown
Author

hi, sorry for waiting
yes I probably will make a new PR'

and yes @Tsafaras, I forgot to change to new style (and also did not know about the --add-noqa argument)

Alright, Let me fix the changes.

Thanks.

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.

3 participants

pFad - Phonifier reborn

Pfad - The Proxy pFad © 2024 Your Company Name. All rights reserved.





Check this box to remove all script contents from the fetched content.



Check this box to remove all images from the fetched content.


Check this box to remove all CSS styles from the fetched content.


Check this box to keep images inefficiently compressed and original size.

Note: This service is not intended for secure transactions such as banking, social media, email, or purchasing. Use at your own risk. We assume no liability whatsoever for broken pages.


Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy