search

jazzband / django-authority

A Django app that provides generic per-object-permissions for Django's auth app and helpers to create custom permission checks.
http://django-authority.readthedocs.org
BSD 3-Clause "New" or "Revised" License
293 stars 58 forks source link

Update and add migration for django 1.8 #46

Closed safwanrahman closed 8 years ago

safwanrahman commented 8 years ago

This change is actually made in kitsune #2781 by @mythmon. Hoping that, it will be merged in this project. @jlward what do you think?

mythmon commented 8 years ago

@safwanrahman one thing to watch out for: I changed some things in ways that are specif to Django 1.8 or Kitsune. For example, there are imports from kitsune here, and it no longer uses the django-authority fixtures. You'll need to fix that before this can be merged.

safwanrahman commented 8 years ago

Oh! Yeah. I did not notice that. Thanks. I will fix that

safwanrahman commented 8 years ago

@jezdez is it possible for you to review the PR?

jezdez commented 8 years ago

@safwanrahman Sorry, can't promise a code review of this due to time constraints

jezdez commented 8 years ago

@safwanrahman caught me on IRC and I took a quick look. This PR looks like three separate things at least:

1) lots of cosmetic code changes to match pep8 and a few other coventions, probably from kitsune being checked with flake8 or similar 2) a new initial migration for Django 1.8 3) some minor fixes for old versions of Django that aren't needed anymore

The actual commits mix up those things and should be separated into separate commits.

Since the tests currently don't pass please drop support for any Django < 1.8 since it's unsupported and Python 2.6 as well while you're at it. You can make that the fourth commit.

safwanrahman commented 8 years ago

updated @jezdez

jezdez commented 8 years ago

@safwanrahman Ah, apologies for not making myself clear, I meant it would be good to have separate commits for the points I listed in my last comment.

You could start by combining e341a83e034e98f410ccc63ec9ce85b88e483833 and 29f2b1a5d3cd58d61d6c5470fc497dd830e81730 and then extract the Django migration into an own commit just like the code that was there to support old Django versions. That could be combined with 9a528c941d6a76bf1fa20c5ae19420d649c84bf3.

safwanrahman commented 8 years ago

@jezdez updated. Can you please review it?

bogdanpetrea commented 8 years ago

Hey, can we have this please?

jezdez commented 8 years ago

nice!