Skip to content
This repository has been archived by the owner on Oct 1, 2022. It is now read-only.

Fix for Django 2.0 #58

Merged
merged 20 commits into from
Mar 22, 2018
Merged

Fix for Django 2.0 #58

merged 20 commits into from
Mar 22, 2018

Conversation

morlandi
Copy link
Contributor

User.is_authenticated is now an attribute; backwards-compatibility support for using it as a method has been removed in Django 2.0.
See:

https://docs.djangoproject.com/en/1.11/ref/contrib/auth/#django.contrib.auth.models.User.is_authenticated

Also, unit tests have been revised, and Django 2.0 added to Tox envlist

@coveralls
Copy link

coveralls commented Feb 24, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling 8092775 on morlandi:master into 5cc459a on semente:master.

@jaap3
Copy link
Collaborator

jaap3 commented Feb 27, 2018

Thank you! I think we can drop support for Django < 1.11 for the next release. Django 1.8 will not be supported after ~April 2018 and older versions of smuggler will work just fine.

Can you update your PR so it only works on Django>=1.11? This means removing the compatibility shim, updating the tox config and removing the conditional imports.

If any conditional imports are still necessary please change them so the "new" import is tried first and the "old" import is in the except clause as a fallback.

Your contribution is much appreciated!

@semente
Copy link
Owner

semente commented Feb 27, 2018

Yea, that would be nice @jaap3!
Thanks @morlandi

@morlandi
Copy link
Contributor Author

@jaap3 thanks for clarifications; I'm willing to improve my PR as suggested, and plan to do it in a week or so

@morlandi
Copy link
Contributor Author

@jaap3 , I was finally able to include your suggestions into my PR.

Dropping support fro Django < 1.11, nor conditional imports neither compatibility shims are needed any more, so the resulting code is cleaner.
Test coverage recovers as well.

For some reason, travis keeps running checks for Django 1.8 and 1.9 configurations, even if I removed them from tox.ini. That's the reason why some tests fail.

Thank you in advance for reviewing my PR.
Please feel free to give advices on how to improve it as needed.

Mario

@jaap3 jaap3 merged commit f893344 into semente:master Mar 22, 2018
@jaap3
Copy link
Collaborator

jaap3 commented Mar 22, 2018

Thank you!

@jaap3 jaap3 mentioned this pull request Mar 22, 2018
Copy link
Owner

@semente semente left a comment

Choose a reason for hiding this comment

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

Releasing 1.0

@@ -131,6 +131,12 @@ Load form (with ``SMUGGLER_FIXTURE_DIR`` configured):
Release notes
=============

Version 0.9.0 (2018-??-??)
Copy link
Owner

Choose a reason for hiding this comment

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

Hey @jaap3, I was thinking in releasing version 0.8 as 1.0 (bug fixes if we ever need on 1.0 branch, then minor bugfixes releases on 1.0.x).

The current master we release as 1.1.

What do you think?

Copy link
Owner

@semente semente Mar 22, 2018

Choose a reason for hiding this comment

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

Actually, better we release it as 0.9. We let it mature than tag it as 1.0 when it is time.

If we ever add new features at future, it should go on 1.1 and the current 0.9.x tag turns in 1.0.

@semente
Copy link
Owner

semente commented Mar 22, 2018

@morlandi @jaap3 awesome! everything looks great! I'm releasing 0.9
Thanks!

@semente
Copy link
Owner

semente commented Mar 22, 2018

@morlandi you have now smuggler 0.9.0 on PyPI :)

@morlandi
Copy link
Contributor Author

Yes sir ! thankyou

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants