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

Django 2.0 support #755

Merged
merged 4 commits into from Feb 20, 2018
Merged

Django 2.0 support #755

merged 4 commits into from Feb 20, 2018

Conversation

rsalmaso
Copy link
Contributor

I'm testing django-wiki, and I need django 2 support :)
It needs https://github.com/benjaoming/django-nyt/pull/55 because I cannot monkey patch as I've done for django-mptt (already django 2 compatible on master) or django-functest (django-functest/django-functest#14)

django-functest has a PR for its support
@benjaoming
Copy link
Member

@rsalmaso was thinking about this a while back -- and to make life easier, I would propose that we release 0.3 as a final release now, without Django 2.0 support.

And then bump master to track a new 0.4 series with just Django 1.11 + 2.0 support?

@rsalmaso
Copy link
Contributor Author

And maybe without py27 support?
It would be fine!

@benjaoming
Copy link
Member

@rsalmaso It's a bit too aggressive to cut 2.7, although I would love to -- the idea behind 1.11 is that it's LTS (so still supported!) and so it helps people to bridge to 2.0... and Python 3 not least!

image

@rsalmaso
Copy link
Contributor Author

rsalmaso commented Jan 22, 2018

@benjaoming ok, in 0.4.x support only django>=1.11? it would simplify the patch a bit (remove the is_authenticated/is_anonymous patch) and some urls imports.
But liniting should be ported to python3, because now it fails :)

@benjaoming
Copy link
Member

ok, in 0.4.x support only django>1.11?

I would argue >=1.11 because as you can see, 1.11 LTS is supported until April 2020. Think about it like this: Since you yourself just decided to migrate to 2.0, others are still in that same process :) 1.11 support can help them arrive with fewer bumps! :)

But liniting should be ported to python3, because now it fails :)

I'll have that done, too!

@rsalmaso
Copy link
Contributor Author

yes, it was >=1.11 and not >1.11 (and now corrected :) )

@rsalmaso
Copy link
Contributor Author

rsalmaso commented Jan 29, 2018

Hi, I've split this PR in 4: #758 #759 #760 (which work with django >= 1.8) and this which explicit add django 2.0 support.
Should be easier this way to review.

@benjaoming
Copy link
Member

Thanks @rsalmaso ! Is there something left in this PR to merge? I guess we need to bump django-nyt once released, right?

@rsalmaso
Copy link
Contributor Author

Yes, it needs a django-nyt compatible with django 2.0 and at least #760 plus probably some cleanup after the 0.3 release.

setup.py Outdated
"six",
"django-mptt>=0.8.6,<0.9",
"django-mptt>=0.8.6",
Copy link
Member

Choose a reason for hiding this comment

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

Removing the upper bounds for django-mptt isn't a good idea. This has historically caused brokenness for deployments.

@benjaoming benjaoming mentioned this pull request Feb 16, 2018
@citxx
Copy link
Contributor

citxx commented Feb 17, 2018

Hi @rsalmaso, thank you a lot for preparing this pull request! I plan to migrate my project to django 2.0 soon and it's great someone has already started making django-wiki ready for this.

Do you still plan to get this pull request merged or should I take over?

@rsalmaso
Copy link
Contributor Author

I'm back.
PR wip updated, currently it needs a django-nyt with django 2 support release.
I see that python2 is currently dropped 👍

@benjaoming
Copy link
Member

@rsalmaso sorry I thought we were waiting for this to release django-nyt with Django 2.0 support: https://github.com/benjaoming/django-nyt/pull/55

@benjaoming
Copy link
Member

Hi @rsalmaso !

Just wanted to say that the current django-nyt master has been released to PyPi as 1.1b1 :)

@codecov
Copy link

codecov bot commented Feb 19, 2018

Codecov Report

Merging #755 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #755   +/-   ##
=======================================
  Coverage   69.87%   69.87%           
=======================================
  Files          99       99           
  Lines        4335     4335           
=======================================
  Hits         3029     3029           
  Misses       1306     1306

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 5918949...ea063be. Read the comment docs.

@rsalmaso
Copy link
Contributor Author

rsalmaso commented Feb 19, 2018

@benjaoming PR updated!

Copy link
Member

@benjaoming benjaoming left a comment

Choose a reason for hiding this comment

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

🎉

@benjaoming benjaoming merged commit 67a30cc into django-wiki:master Feb 20, 2018
@rsalmaso rsalmaso deleted the django2 branch February 20, 2018 14:50
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

3 participants