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.

Sign up for GitHub

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

Jump to bottom

add django 3.2 and 4.0 support #112

Merged
merged 2 commits into from
Oct 6, 2021
Merged

add django 3.2 and 4.0 support #112

merged 2 commits into from
Oct 6, 2021

Conversation

umarmughal824
Copy link
Contributor

@umarmughal824 umarmughal824 commented Sep 23, 2021 •
edited
Loading

Add django 3.2 and 4.0 support

I am facing the following warning while upgrading the django to 3.2 which take the default variable for DEFAULT_AUTO_FIELD ref

web_1     | robots.Rule: (models.W042) Auto-created primary key used when not defining a primary key type, by default 'django.db.models.AutoField'.
web_1     | 	HINT: Configure the DEFAULT_AUTO_FIELD setting or the AppConfig.default_auto_field attribute to point to a subclass of AutoField, e.g. 'django.db.models.BigAutoField'.
web_1     | robots.Url: (models.W042) Auto-created primary key used when not defining a primary key type, by default 'django.db.models.AutoField'.
web_1     | 	HINT: Configure the DEFAULT_AUTO_FIELD setting or the AppConfig.default_auto_field attribute to point to a subclass of AutoField, e.g. 'django.db.models.BigAutoField'.

So in lieu of that I am facing the error in my CI/CD that is

[TEST SUITE]./scripts/test/detect_missing_migrations.sh
Error: one or more migrations are missing
Migrations for 'robots': /opt/hostedtoolcache/Python/3.7.5/x64/lib/python3.7/site-packages/robots/migrations/0002_auto_20210923_1203.py - Alter field id on rule - Alter field id on url

@umarmughal824 umarmughal824 mentioned this pull request Sep 23, 2021
Merged
3 tasks
@umarmughal824
Copy link
Contributor Author

umarmughal824 commented Sep 23, 2021 •
edited
Loading

@jezdez could you please take a look at that PR.

@umarmughal824 umarmughal824 changed the title fix auto field warning for django 3.2 add django 3.2 support Sep 23, 2021
@codecov
Copy link

codecov bot commented Sep 25, 2021 •
edited
Loading

Codecov Report

Merging #112 (91adf53) into master (be2e781) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #112   +/-   ##
=======================================
  Coverage   79.85%   79.85%           
=======================================
  Files           7        7           
  Lines         134      134           
  Branches       13       13           
=======================================
  Hits          107      107           
  Misses         20       20           
  Partials        7        7

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be2e781...91adf53. Read the comment docs.

smithdc1
smithdc1 reviewed Sep 25, 2021
View reviewed changes
tox.ini
@@ -2,7 +2,7 @@
envlist =
# list of supported Django/Python versions:
# https://docs.djangoproject.com/en/3.1/faq/install/#what-python-version-can-i-use-with-django
py{36,37,38,39}-dj{22,30,31}
py{36,37,38,39}-dj{22,30,31,32}
py{38,39}-djmain
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add Django 4.0 support as well while we're here?

Copy link
Contributor Author

@umarmughal824 umarmughal824 Sep 27, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I didn't get any chance to explore Django 4 yet but I will see if it's a quick fix.

Copy link
Contributor Author

@umarmughal824 umarmughal824 Sep 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@smithdc1 Django 4.0 is not yet final so better to wait for the final release which is due in December.
https://code.djangoproject.com/wiki/Version4.0Roadmap

Copy link
Contributor

Choose a reason for hiding this comment

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

It's final in terms of features and changes. The only patches being applied to the 4.0 branch are bug fixes. Folk are adding the classifier to their packages already. See:

https://pypi.org/search/?c=Framework+%3A%3A+Django+%3A%3A+4.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.

@smithdc1 I'm waiting for your review, thanks

smithdc1
smithdc1 reviewed Sep 25, 2021
View reviewed changes
tox.ini Outdated Show resolved Hide resolved
@umarmughal824
Copy link
Contributor Author

@smithdc1 your feedback is addressed so it's ready for review again.

@umarmughal824 umarmughal824 changed the title add django 3.2 support add django 3.2 and 4.0 support Sep 29, 2021
@umarmughal824
Copy link
Contributor Author

umarmughal824 commented Sep 29, 2021 •
edited
Loading

@smithdc1 I have added 4.0 support as well and it seems to be working fine. Here are screenshots of running tests

Screenshot 2021-09-29 at 14 41 11

Screenshot 2021-09-29 at 14 40 08

smithdc1
smithdc1 reviewed Oct 4, 2021
View reviewed changes
Copy link
Contributor

@smithdc1 smithdc1 left a comment

Choose a reason for hiding this comment

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

Thanks for this 👍

I think we're nearly there. I've re-run the tests but expecting to see them fail as Django 4.0 should fail with Python 3.6&3.7.

README.rst Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
@umarmughal824 umarmughal824 requested a review from smithdc1 October 5, 2021 07:24
@smithdc1
Copy link
Contributor

smithdc1 commented Oct 6, 2021

@umarmughal824

I'm sorry, I forgot about the classifiers when looking at this before. 🤦‍♂️

They are in the setup.py file. Could you adjust?

Apologies.

@umarmughal824
Copy link
Contributor Author

umarmughal824 commented Oct 6, 2021 •
edited
Loading

@smithdc1 I have already added django 3.2 and 4 in the classifiers too. I am not sure, what do you mean by that?
https://github.com/jazzband/django-robots/pull/112/files#diff-60f61ab7a8d1910d86d9fda2261620314edcae5894d5aaa236b821c7256badd7L47

@smithdc1 smithdc1 merged commit 70da760 into jazzband:master Oct 6, 2021
@smithdc1
Copy link
Contributor

smithdc1 commented Oct 6, 2021

@umarmughal824 -- Sorry, having a bad day. Thanks for the contribution. 🙏

@umarmughal824 umarmughal824 deleted the umar/support-django32 branch October 7, 2021 05:06
@umarmughal824
Copy link
Contributor Author

@umarmughal824 -- Sorry, having a bad day. Thanks for the contribution. 🙏

feel better

@tony
Copy link
Member

tony commented Jan 6, 2022

@umarmughal824 This is prereleased at 5.0b1 if you'd like to give it a go. Would like some testing before final is published

@tony tony mentioned this pull request Jan 6, 2022
Closed
This was referenced Apr 23, 2022
Closed
Closed
@tony
Copy link
Member

tony commented Jun 1, 2022

@umarmughal824 and @smithdc1 There is an issue that came up due to this PR in #124, can you take a closer look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smithdc1 smithdc1 Awaiting requested review from smithdc1

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@umarmughal824 @smithdc1 @tony