Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade Django Rest Framework #1703

Merged
merged 45 commits into from May 8, 2018
Merged

Conversation

jmbredal
Copy link
Collaborator

@jmbredal jmbredal commented Apr 26, 2018

Upgrade Django Rest Framework to at least support Django 1.8

As tests are passing now I consider this complete. For the reviewers - verify model-changes and take a look at the tests to see if they are sane.

hmpf and others added 30 commits April 26, 2018 13:04
Incomplete list of things that have changed in `response.data`:

* Returns a list (ReturnList) not a `dict('results', [results])`
* Returns an iso-formatted string instead of a `datetime` for dates
@jmbredal jmbredal changed the title wip: Upgrade Django Rest Framework Upgrade Django Rest Framework May 3, 2018
@jmbredal jmbredal requested review from lunkwill42 and hmpf May 3, 2018 08:01
@@ -652,6 +653,26 @@ def get_times(request):
return starttime, endtime



Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, moving code to where it belongs

Copy link
Contributor

@hmpf hmpf left a comment

Choose a reason for hiding this comment

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

Thumbs up! Great to have tests for it too.

Copy link
Member

@lunkwill42 lunkwill42 left a comment

Choose a reason for hiding this comment

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

This looks fine to me except for:

  1. Minor nitpick about naming
  2. NOTES.rst needs a section about which dependencies a user needs to upgrade to which versions for this stuff to work.

from .models import LogEntry


class LGFKRelatedField(serializers.RelatedField):
Copy link
Member

Choose a reason for hiding this comment

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

I know LegacyGenericForeignKeyRelatedField is a long type, but I don't appreciate this type of mysterious abbreviations. Its only redeeming factor is that the scope is short: It's only used in the next class...

@lunkwill42 lunkwill42 mentioned this pull request May 8, 2018
@lunkwill42 lunkwill42 added this to the 4.9.0 milestone May 8, 2018
@lunkwill42 lunkwill42 merged commit 16e4da6 into Uninett:master May 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants