search

jazzband / django-embed-video

Django app for easy embedding YouTube and Vimeo videos and music from SoundCloud.
http://django-embed-video.rtfd.org
MIT License
383 stars 137 forks source link

Drop support Django 1.11 and add Python3.9 support. #138

Closed hramezani closed 3 years ago

smithdc1 commented 3 years ago

A couple of the requirements files have Django pinned to version 1.5. Not sure if adjusting these (could remove the pin entirely?) is out of scope here.

hramezani commented 3 years ago

A couple of the requirements files have Django pinned to version 1.5. Not sure if adjusting these (could remove the pin entirely?) is out of scope here.

Thanks, @smithdc1 . I missed them. I updated them to Django>=2.2.

mgrdcm commented 3 years ago

Look at all that glorious removed code! 😍

mgrdcm commented 3 years ago

How would you feel about adding in the supported Django versions shield too since we're adding the Python one in this PR? https://img.shields.io/pypi/djversions/django-embed-video.svg

hramezani commented 3 years ago

How would you feel about adding in the supported Django versions shield too since we're adding the Python one in this PR? https://img.shields.io/pypi/djversions/django-embed-video.svg

Good idea :+1: Done!

mgrdcm commented 3 years ago

This looks great to me. One question - since we are supporting back to Django 2.2 and that supports Python 3.5 should we add 3.5 to the support/tests even though 3.5 is EOL'd as of September 2020? Since it's an LTS I'd think we should but I won't fight over it ;)

mgrdcm commented 3 years ago

Oh and of course need to make sure the release that includes this is a breaking/major change version so like 2.0.0 right?

smithdc1 commented 3 years ago

It seems this project hasn't supported python 3.5 for a while. Would seem odd to add it now as it's eol. Agree generally that Django 2.2 still supports py3.5 so it's good if packages can follow this.

Not sure about need for it to be a major version bump. I'm not seeing this approach across many of the Django packages. But if that's the policy for this package it should be followed. 🤷

hramezani commented 3 years ago

@mgrdcm About Python3.5, it would be good to read this comment from @aleksihakli. I think we can ignore Python3.5 unless there is a strong reason to keep it.

hramezani commented 3 years ago

Not sure about need for it to be a major version bump. I'm not seeing this approach across many of the Django packages. But if that's the policy for this package it should be followed.

Agree with @smithdc1 :+1:

mgrdcm commented 3 years ago

@mgrdcm About Python3.5, it would be good to read this comment from @aleksihakli. I think we can ignore Python3.5 unless there is a strong reason to keep it.

Agreed - sounds good to me on not adding support for it now since it's EOL'd!

Not sure about need for it to be a major version bump. I'm not seeing this approach across many of the Django packages. But if that's the policy for this package it should be followed. 🤷

Good point! I don't see that this project explicitly requires it but I'd think that dropping support for a major Django version (even if EOL'd) would qualify as breaking. I don't see a reason not to make it a breaking change in any case. But not gonna fight about this either!

Thanks to @hramezani for this PR and everyone who's contributing to this project!