chrisdev / django-wagtail-feeds

Adds support for RSS and JSON Feeds to your Wagtail CMS Projects
MIT License
59 stars 20 forks source link

Upgrade project to Django and Wagtail v2 #46

Closed mtn closed 6 years ago

mtn commented 6 years ago

It turned out to be easier than expected! Thanks @Parbhat 🎈

python runtest.py passes with Django v2, python3, wagtail v2.

mtn commented 6 years ago

I believe the tox tests should be modified, because they don't test the correct environments anymore. Is this right?

Parbhat commented 6 years ago

@mtn Great work. Correct we need to remove the python 2 related Travis tests. We probably need only Django 1.11.x and 2.0 on Python 3.4, 3.5 and 3.6

mtn commented 6 years ago

@Parbhat Got it. Can I just modify the tox.ini? Haven't used tox before.

Parbhat commented 6 years ago

@mtn Sure. Feel free to change the tox.ini and .travis.yml.

mtn commented 6 years ago

Hm, I found this to be fairly confusing. Build 67 passes without any 3.6 tests, but I can't make sense of how the tox environments are being run against different python versions. For example, in build 67, python 3.5 is run with a tox environment using 3.4? It might be clearest to look at the most recent build failure where I tried to add 3.6 tests but ended up with a bunch of configurations I didn't expect.

mtn commented 6 years ago

It looks like a combination I didn't expect is still being run, even after fixing the comma. File here.

Thanks for all the help!

Parbhat commented 6 years ago

@mtn Great start. .travis.yml also need to be updated to remove Django10 and add Django 2.

mtn commented 6 years ago

Hm, alright. Not quite working still -- I'll revisit this at some point. Let me know if anything stands out.

mtn commented 6 years ago

@Parbhat Is there any chance you'd be able to take a look and let me know if you have any suggestions on how to modify the tox or travis config files? Thanks for all the help!

Parbhat commented 6 years ago

@mtn The errors on Travis are all InterpreterNotFound errors like

ERROR:   py35-django20: InterpreterNotFound: python3.5

From the docs here, we need to update env value in .travis.yml to

matrix:
  include:
  - env: TOXENV=py34-django20
     python: 3.4
...
...
  -env: TOXENV=py35-django11
  python: 3.5

Specify the python interpreter along with TOXENV. An example can be seen in Wagtail project.

Let's try to do these changes.

Also, it seems that we should not drop support for Wagtail 1.13.1 as we can handle it with try / except block to fall back on the old import paths. We can update feeds.py like

from wagtail import VERSION as WAGTAIL_VERSION

if WAGTAIL_VERSION >= (2, 0):
    from wagtail.core.models import Site
    from wagtail.core.rich_text import expand_db_html
else:
    from wagtail.wagtailcore.models import Site
    from wagtail.wagtailcore.rich_text import expand_db_html

We can skip the tests for Wagtail 1.13.1

Please post here how it goes. Thanks.

mtn commented 6 years ago

Hm, unsure why travis isn't triggering a build, but at least for the versions of python I have locally (3.6) the tests are passing. I haven't added the line to maintain version 1.13.1 support yet. Will poke around a bit more before wandering off 🎈

Edit: Ah good, I found an error in the travis interface for the last commit. Edit 2: Tests seem good to go, let me know if you think otherwise!