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

Test Django 4.1 and remove upper version bound #1203

Merged
merged 3 commits into from
Sep 6, 2022

Conversation

adamchainz
Copy link
Contributor

Fixes #1202

Description of the Change

Remove upper version bound, as I recommended on the ticket.

Checklist

  • PR only contains one change (considered splitting up PR)
  • unit-test added
  • documentation updated
  • CHANGELOG.md updated (only for user relevant changes)
  • author name in AUTHORS

@adamchainz
Copy link
Contributor Author

@n2ygk @Andrew-Chen-Wang , since you worked on #1175 which added this version bound.

@codecov
Copy link

codecov bot commented Sep 5, 2022

Codecov Report

Merging #1203 (1b7a08b) into master (1b7a08b) will not change coverage.
The diff coverage is n/a.

❗ Current head 1b7a08b differs from pull request most recent head 6bc1646. Consider uploading reports for the commit 6bc1646 to get more accurate results

@@           Coverage Diff           @@
##           master    #1203   +/-   ##
=======================================
  Coverage   96.85%   96.85%           
=======================================
  Files          31       31           
  Lines        1813     1813           
=======================================
  Hits         1756     1756           
  Misses         57       57           

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

@@ -32,7 +32,7 @@ zip_safe = False
# jwcrypto has a direct dependency on six, but does not list it yet in a release
# Previously, cryptography also depended on six, so this was unnoticed
install_requires =
django >= 2.2, <= 4.1
django >= 2.2, != 4.0.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

restoring the block on 4.0.0 which had a bug, as noted here:

#1039 (review)

Copy link
Member

@Andrew-Chen-Wang Andrew-Chen-Wang left a comment

Choose a reason for hiding this comment

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

@n2ygk I don't think an upper bound is necessary either. also LGTM

@adamchainz adamchainz changed the title Remove upper version bound on Django 4.1 Test Django 4.1 and remove upper version bound Sep 5, 2022
n2ygk
n2ygk previously requested changes Sep 6, 2022
Copy link
Member

@n2ygk n2ygk left a comment

Choose a reason for hiding this comment

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

Please leave the upper bound test in. Making it open-ended is doing a disservice to library users. We just need to be more on top of updating the versions that we test against and resolving discrepancies.

setup.cfg Show resolved Hide resolved
@n2ygk
Copy link
Member

n2ygk commented Sep 6, 2022

@Andrew-Chen-Wang > @n2ygk I don't think an upper bound is necessary either. also LGTM

I kinda disagree given the tox tests are failing with djmain. But 2 against 1 wins I guess.

@n2ygk n2ygk dismissed their stale review September 6, 2022 17:30

I give in

@n2ygk n2ygk merged commit 235e3f8 into jazzband:master Sep 6, 2022
@aaronmader
Copy link

Any idea when we can expect the next release to pypi, including this fix?

@adamchainz adamchainz deleted the remove_upper_version_bound branch September 26, 2022 15:18
@n2ygk n2ygk added this to the 2.2.0 milestone Oct 18, 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.

Incorrect upper bound prevents use of Django 4.1.1+
4 participants