Hi, On Wed, 14 Dec 2016, efkin wrote: > I decided to keep DRF 2.4.3. But i'm happy to change it if you prefer. > I have no big reasons to prefer one or the other one, so i fallback on > the fact that if it's on stable i go for it.
stretch is coming in a few months and we are not using Django from stable either, so I would suggest to use directly DRF 3.4. > > Reported-by: Paul Wise <p...@debian.org> > > Done. You are still missing: s/_/-/ > Btw sorry for the short description of my last email, i try to > compensate it now: What about using the API self-documentation feature? http://www.django-rest-framework.org/topics/documenting-your-api/#self-describing-apis That will also force you to choose good names to your views. > diff --git a/distro_tracker/api/tests.py b/distro_tracker/api/tests.py > index 7ce503c..255bbb8 100644 > --- a/distro_tracker/api/tests.py > +++ b/distro_tracker/api/tests.py > @@ -1,3 +1,104 @@ > -from django.test import TestCase > +# Copyright 2014-2016 The Distro Tracker Developers Copyright 2016 only for new code. :) Applies on all files. > --- /dev/null > +++ b/distro_tracker/api/tracker_urls.py > @@ -0,0 +1,29 @@ > +# Copyright 2014-2016 The Distro Tracker Developers > +# See the COPYRIGHT file at the top-level directory of this distribution and > +# at https://deb.li/DTAuthors > +# > +# This file is part of Distro Tracker. It is subject to the license terms > +# in the LICENSE file found in the top-level directory of this > +# distribution and at https://deb.li/DTLicense. No part of Distro Tracker, > +# including this file, may be copied, modified, propagated, or distributed > +# except according to the terms contained in the LICENSE file. > + > +from django.conf.urls import url > + > +from .views import ActionItemListAPIView > +from .views import ActionItemDetailAPIView > + > + > +urlpatterns = [ > + url(r'^api/v1/action-items/?$', > + ActionItemListAPIView.as_view(), > + name='dtracker-api-v1-action-items'), > + url(r'^api/v1/action-items/(?P<pk>[0-9]+)/?$', > + ActionItemDetailAPIView.as_view(), > + name='dtracker-api-v1-action-items'), > +] Please don't use the same "name" for both URL. Use something like "...-action-item-list" and "...-action-item-detail". Also, after having looked at the DRF doc, we should not hardcode the versioning at this level, we should rather use NamespaceVersioning: http://www.django-rest-framework.org/api-guide/versioning/#namespaceversioning > +class ActionItemListAPIView(generics.ListAPIView): Shouldn't we merge both objects with a single ReadOnlyModelViewSet? http://www.django-rest-framework.org/api-guide/viewsets/#readonlymodelviewset Also you should name the object "ActionItemListView" as the online documentation only strips the "View" suffix. > + """ > + List all ActionItem instances. > + """ Here you must explain the GET query parameter that we support. > + def get_queryset(self): > + queryset = ActionItem.objects.all() > + package_name = self.request.GET.get('package_name', None) package_name is a required abstraction at the database level but it makes no sense for end users. I would just use "package" as the query parameter. Also, one of the early design decision seems to be whether we use an hyperlinked serializer or not. I don't have a strong opinion on that but it makes it easier to navigate interactively in the data through the browser. So unless we have a reason not to, I believe it would be nice to use it: http://www.django-rest-framework.org/tutorial/5-relationships-and-hyperlinked-apis/#hyperlinking-our-api But even if we use the hyperlinked version of the serializer, I would still include the "id" attribute everywhere. Cheers, -- Raphaël Hertzog ◈ Debian Developer Support Debian LTS: http://www.freexian.com/services/debian-lts.html Learn to master Debian: http://debian-handbook.info/get/
