gintas / django-picklefield

A pickled object field for Django
MIT License
180 stars 47 forks source link

Updated DEFAULT_PROTOCOL to 3. #50

Closed carltongibson closed 4 years ago

carltongibson commented 4 years ago

Protocol version 3 was added in Python 3.0. It has explicit support for bytes objects and cannot be unpickled by Python 2.x. This was the default protocol in Python 3.0–3.7.

Not sure it dropping PY2 is on the cards now that it's EOL but, if so I think it would be fair to bump the default protocol version.

WIP because they'll be lots that breaks on tox/travis. I will check back on that later.

coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 98.901% when pulling 5bc1900d4582d197bc67707a70aff3e633d8e02c on carltongibson:patch-1 into 44dfeb9c84bc00d6a9debfacffcf3ba207ffcefd on gintas:master.

charettes commented 4 years ago

Hey @carltongibson 👋

Looks like it only breaks the Python 2 build which is expected. Given tests already pass on Python 2 simply removing tested support (tox.ini, .travis.yml, setup.py) and adding a section for in the README.rst release notes under a new 3.0 section would do.

Removal of Python support code can be done in a following PR.

carltongibson commented 4 years ago

Hi @charettes 👋

I dropped PY2 in a separate commit, if you're happy to take that.

If you're happy with these I can rebase and drop 1.11 too whilst I'm here.

charettes commented 4 years ago

If you're happy with these I can rebase and drop 1.11 too whilst I'm here.

Totally open to that if you're up to it! I can take care of releasing 3.0 afterwards. It looks like the tox.ini still has references to Django 2.0 and 2.1 if you want to remove these as well.

carltongibson commented 4 years ago

OK, I'll add it here. 10 mins.

charettes commented 4 years ago

Looks like Travis is taking a ☕ break.

carltongibson commented 4 years ago

I don't blame it. 😀

I'll check back on this later. Hopefully the lint builds work with python3.6, apparently they couldn't find python3.7...

charettes commented 4 years ago

We could run it against 3.7 (or even 3.8) if the .travis-ci.yml file is adjusted

https://github.com/gintas/django-picklefield/blob/44dfeb9c84bc00d6a9debfacffcf3ba207ffcefd/.travis.yml#L17-L18

carltongibson commented 4 years ago

It seems to be OK now. https://travis-ci.org/github/gintas/django-picklefield/builds/695030386

charettes commented 4 years ago

Perfect thanks!