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

chore: use pyproject.toml instead of setup.cfg #138

Merged
merged 2 commits into from Oct 4, 2023

Conversation

GabDug
Copy link
Contributor

@GabDug GabDug commented Sep 7, 2023

Small DX/packaging MR, following latest PEPs.

Using pyproject.toml and being PEP621-compliant will also make it easier to integrate with other dev-tools, like PDM.

@GabDug
Copy link
Contributor Author

GabDug commented Sep 7, 2023

Unfortunately, it seems this approach does not work well for Python 3.6.

What are you thoughts on increasing the minimum Python version supported? Security support for 3.6 stopped on 23 Dec 2021, for 3.7 it was 2 months ago, lots of tools and libs now target 3.8-3.12.

@kytta
Copy link
Member

kytta commented Sep 7, 2023

What are you thoughts on increasing the minimum Python version supported? Security support for 3.6 stopped on 23 Dec 2021, for 3.7 it was 2 months ago, lots of tools and libs now target 3.8-3.12.

Since Django 3.2 still supports Python 3.6, and we still support Django 3.2, we keep supporting Python 3.6 for the possible LTS users. So, we won't update the minimal version until 01 Apr 2024

@GabDug
Copy link
Contributor Author

GabDug commented Sep 7, 2023

Understood.
Supporting 3.6 brings additional issues, as tox 4 only supports Python 3.7+ and latest setuptools only support 3.8+, while the CI only includes one Python version per job, so I'll close this PR.

@GabDug GabDug closed this Sep 7, 2023
@kytta
Copy link
Member

kytta commented Sep 7, 2023

Supporting 3.6 brings additional issues, as tox 4 only supports Python 3.7+ and latest setuptools only support 3.8+, while the CI only includes one Python version per job, so I'll close this PR.

Theoretically we could include two Pythons per CI job: 3.11 running tox and whatever the matrix version is to run the tests. tox v4 supports Python 3.6 for the environments, but not for the "host".

I haven't had time to take a look at how this can be done; if you're interested in taking a look at this, I'd be happy 😁

P.S. Sorry for accidentally editing your comment, I've missclicked 😂

@GabDug
Copy link
Contributor Author

GabDug commented Sep 7, 2023

I agree that it should be doable, but I'm not sure it's worth it, it will probably add a lot of complexity for a Python version that's not gonna last a lot more!

I respect your commitment to support legacy Python versions, but to be honest, I don't share it: I believe that people that did not upgrade neither Python nor Django won't upgrade their dependencies.

Looking at how others projects handle this issue, I found the way django-taggit handles this, and it's way simpler 😂

Still I'll let you know if I find some time to look into the issue :)

@kytta
Copy link
Member

kytta commented Sep 7, 2023

Looking at how others projects handle this issue, I found the way django-taggit handles this, and it's way simpler 😂

That is genius, how didn't I come up with this earlier?!

@GabDug GabDug reopened this Sep 7, 2023
@codecov
Copy link

codecov bot commented Sep 7, 2023

Codecov Report

Merging #138 (3951357) into master (907ed10) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #138   +/-   ##
=======================================
  Coverage   91.07%   91.07%           
=======================================
  Files           6        6           
  Lines         325      325           
  Branches       75       75           
=======================================
  Hits          296      296           
  Misses         13       13           
  Partials       16       16           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@GabDug
Copy link
Contributor Author

GabDug commented Sep 7, 2023

Simpler than anticipated, also removed Django 4.0.

@GabDug
Copy link
Contributor Author

GabDug commented Sep 15, 2023

Hey @kytta,

Small bump to the PR in case you have not seen the latest changes.
If you have, sorry for the notification! There's no rush here :)

Have a nice day!

@kytta
Copy link
Member

kytta commented Sep 20, 2023

I hope I'll get to review this properly till the end of the week, otherwise you'll have to wait till October :D

@kytta kytta self-requested a review September 20, 2023 17:30
@kytta kytta merged commit 80fa518 into jazzband:master Oct 4, 2023
35 checks passed
@GabDug GabDug deleted the chore/pep-621 branch October 4, 2023 10:06
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.

None yet

2 participants