search

django-nonrel / djangotoolbox

Django tools for building nonrel backends
BSD 3-Clause "New" or "Revised" License
200 stars 125 forks source link

Issue #55 - Database Backend Support For Django 1.7 #62

Closed sethdenner closed 9 years ago

sethdenner commented 9 years ago

I got a start on this tonight. Not ready for merge yet.

I just took @kavdev's implementation for _decode_child and refactored it into a sub method definition called decode_child and did the same with the _decode_child implementation from master and put it in a second decode_child definition. Currently I'm using @kavdev's for django.VERSION >= (1, 7) and using the deffinition from master in all other cases.

I still need to fingure out the proper way to get the annotation parameter and verify that the tests pass before this gets mereged.

sethdenner commented 9 years ago

OK This is ready to be reviewed now. Is this ready for a merge? Normally I would rebase everything into one commit but I wanted to keep @kavdev's initial commit since he did a bulk of the work in figuring out how to query this new API.

aburgel commented 9 years ago

Great stuff @sethdenner! Can you update this to follow the existing coding style? Its a bit hard to see what is new code and what is reformatting.

Also, do these test suites still pass?

sethdenner commented 9 years ago

Would you prefer that I leave the original formatting and to hell with pep-8? Or would adding comments indicating where the code was refactor from be better?

kavdev commented 9 years ago

There were only a few pep8 errors. In my experience, it's safe to --ignore=E501,E114,E116,E128.

Here's the atomic version of @sethdenner's commit https://github.com/kavdev/djangotoolbox/commit/03cb2429901a4025db2339dcabe4c760586755d2

The tests won't pass with python 3. #60 seems to have a solution, but I haven't taken a deep look at it.

aburgel commented 9 years ago

Closing because this was merged in #63