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

make .pre-commit-config.yaml similar to poetry repo #271

Merged
merged 1 commit into from
Jan 25, 2022

Conversation

finswimmer
Copy link
Member

  • sort and update pre-commit repos to be similar to the one used in poetry.
  • add some more pre-commit ids
  • add yesqarepo
  • add pygrep repo
  • use experimental_string_processing parameter for `black

Relates to: python-poetry/poetry#4776

@finswimmer finswimmer requested a review from a team January 24, 2022 19:41
@finswimmer finswimmer force-pushed the sort-pre-commit-hooks branch 7 times, most recently from bf46a8c to f262d3a Compare January 24, 2022 20:26
.pre-commit-config.yaml Outdated Show resolved Hide resolved
@finswimmer finswimmer force-pushed the sort-pre-commit-hooks branch from f262d3a to dccb44a Compare January 25, 2022 05:04
@finswimmer finswimmer requested a review from radoering January 25, 2022 05:06
@finswimmer finswimmer force-pushed the sort-pre-commit-hooks branch from dccb44a to 479f78a Compare January 25, 2022 05:11
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Comment on lines 40 to +42
try:
# Only for Python 3.3+
import lzma # noqa
import lzma # noqa: F401
Copy link
Contributor

Choose a reason for hiding this comment

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

is python <3.3 compat still required here?

if we need it, we should use

if sys.version_info >= (3, 3):
    import lzma
    ....

or similar

Copy link
Member Author

Choose a reason for hiding this comment

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

I would try to avoid logic change to the code within this PR, because its only about formatting. I think this is better handled in #263 for example.

Copy link
Contributor

Choose a reason for hiding this comment

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

ping @tony

Copy link
Contributor

@tony tony Jan 25, 2022

Choose a reason for hiding this comment

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

I think try/catch still makes sense here. It's true was originally an underlying version constraint.

lzma is optionally built in. You can compile CPython without lzma

Could not figure out the flags, Not having the library seems to do it:

❯ sudo apt remove liblzma-dev
❯ make distclean
❯ ./configure
❯ make -j8
❯ ./python -V
Python 3.11.0a4+

❯ ./python -c 'import lzma'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/user/projects/c/cpython/Lib/lzma.py", line 27, in <module>
    from _lzma import *
    ^^^^^^^^^^^^^^^^^^^
ModuleNotFoundError: No module named '_lzma'

Copy link
Contributor

Choose a reason for hiding this comment

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

that makes sense

Comment on lines 12 to +15
try:
FileNotFoundError
except NameError:
FileNotFoundError = IOError # noqa
FileNotFoundError = IOError
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto here - can we use if sys.version_info > ...?

Copy link
Member Author

Choose a reason for hiding this comment

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

same as above

Copy link
Contributor

Choose a reason for hiding this comment

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

ping @tony

Copy link
Contributor

Choose a reason for hiding this comment

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

In #263 I pushed a change removing this

@neersighted neersighted merged commit d9ed333 into python-poetry:master Jan 25, 2022
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.

5 participants