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

Add Python 3.12 to test matrix and add classifier #689

Merged
merged 2 commits into from
Oct 25, 2024

Conversation

foarsitter
Copy link
Contributor

Everything seems fine for Python 3.12 and no change are required.

Copy link

codecov bot commented Oct 31, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.2%. Comparing base (f23b06d) to head (6321750).
Report is 4 commits behind head on master.

Additional details and impacted files
@@          Coverage Diff           @@
##           master    #689   +/-   ##
======================================
  Coverage    63.2%   63.2%           
======================================
  Files          43      43           
  Lines        3231    3231           
  Branches      244     244           
======================================
  Hits         2039    2039           
  Misses       1175    1175           
  Partials       17      17           
Flag Coverage Δ
mypy 37.9% <ø> (ø)
tests 90.3% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copy link
Member

@WisdomPill WisdomPill left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -90,7 +95,7 @@ jobs:
- name: Install dependencies
run: |
python -m pip install --upgrade pip
python -m pip install --upgrade "tox<4" tox-gh-actions
python -m pip install --upgrade "tox<4" tox-gh-actions setuptools
Copy link
Member

Choose a reason for hiding this comment

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

why setuptools was added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

setuptools is no longer bundled with Python 3.12

setup.cfg Outdated Show resolved Hide resolved
@foarsitter
Copy link
Contributor Author

Seems like we need to update pytest to > 7.0 in order to get the tests working for Python 3.12. My initial statement was a little to harsh.

@WisdomPill
Copy link
Member

I am afraid it is a bit early to work add python3.12, it might fail until redis/redis-py#2979

In the meantime #697 will add support for pytest>=7

@chayim
Copy link

chayim commented Jan 10, 2024

@WisdomPill We have an issue with async SSL in 3.12. But this should release in the 5.x final.

@WisdomPill
Copy link
Member

thanks for the heads up @chayim! 😉

@foarsitter
Copy link
Contributor Author

We need to wait for the 5.1 release of redis-py before this can be merged, right?

Copy link
Member

@WisdomPill WisdomPill left a comment

Choose a reason for hiding this comment

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

please rebase and fix your PR :)

setup.cfg Outdated Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
Copy link
Member

@WisdomPill WisdomPill left a comment

Choose a reason for hiding this comment

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

sorry, just noticed, could you also put python 3.12 for pre-release tests?

@WisdomPill
Copy link
Member

anyway, LGTM! And thank you very much for your contribution

@foarsitter
Copy link
Contributor Author

@WisdomPill thanks for your patience :) Feel free to push to this branch!

Copy link
Member

@WisdomPill WisdomPill left a comment

Choose a reason for hiding this comment

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

LGTM!

@WisdomPill WisdomPill merged commit 0f3f824 into jazzband:master Oct 25, 2024
23 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.

3 participants