-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
Stop ignoring test outcome for Django 3.2 #7927
Stop ignoring test outcome for Django 3.2 #7927
Conversation
ce8665c
to
c0fd2a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
CI: 👀
There is still one failure from a warning raised in django-guardian. @smithdc1 already made a pre-emptive PR there back in October - django-guardian/django-guardian#721 - it's just not released yet. We could ignore that one warning. |
Ah, yes, the periodic Guardian is not updated shuffle™ — Yes, skip/ignore as needed. |
It seems guardian is only installed in tests as an example of a permissions backend that supports object-level permissions. We could also rewrite the tests to not rely on it. |
@adamchainz — I think if you have a burst of energy for that it would save this issue coming up… (it's not often but...) |
I am done for the day here but will come back to this and evaluate my energy levels then. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering how Django 3.2 was being specified, and since you're touching this file does it make sense to also adjust the django32
specification from Django>=3.2a1,<4.0
to Django>=3.2,<4.0
?
I would have commented directly on the line, but GitHub doesn't allow that.
@adamchainz It may not be worth the cycles to rewrite the tests here right now. For reference here's a previous iteration 524a28c (you can see it's not a frequent issue) Up to you. 🙂 🥇 |
@adamchainz @carltongibson A new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @johnthagen
@adamchainz(-bot) rebase & merge 😀
It has been released!
c0fd2a7
to
cfa76bd
Compare
It has been released!