| Message ID | 1479574288-24171-5-git-send-email-stephen@that.guru |
|---|---|
| State | Superseded |
| Headers | show |
LGTM! The repetition is a bit annoying, but I agree that the old code was worse. I assume that the 255 char limit is what the model? Reviewed-by: Daniel Axtens <dja@axtens.net> Regards, Daniel Stephen Finucane <stephen@that.guru> writes: > This is apparently the correct way to rename fields, and will ensure a > future API version that supports writes will work correctly. > > Signed-off-by: Stephen Finucane <stephen@that.guru> > Cc: Andy Doan <andy.doan@linaro.org> > --- > patchwork/api/project.py | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/patchwork/api/project.py b/patchwork/api/project.py > index 17f4b3c..9e5d2aa 100644 > --- a/patchwork/api/project.py > +++ b/patchwork/api/project.py > @@ -17,6 +17,7 @@ > # along with Patchwork; if not, write to the Free Software > # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > > +from rest_framework.serializers import CharField > from rest_framework.serializers import HyperlinkedModelSerializer > > from patchwork.api import PatchworkPermission > @@ -25,17 +26,15 @@ from patchwork.models import Project > > > class ProjectSerializer(HyperlinkedModelSerializer): > - def to_representation(self, instance): > - data = super(ProjectSerializer, self).to_representation(instance) > - data['link_name'] = data.pop('linkname') > - data['list_email'] = data.pop('listemail') > - data['list_id'] = data.pop('listid') > - return data > + # TODO(stephenfin): These should be renamed at the model layer > + link_name = CharField(max_length=255, source='linkname') > + list_id = CharField(max_length=255, source='listid') > + list_email = CharField(max_length=255, source='listemail') > > class Meta: > model = Project > - fields = ('url', 'name', 'linkname', 'listid', 'listemail', 'web_url', > - 'scm_url', 'webscm_url') > + fields = ('url', 'name', 'link_name', 'list_id', 'list_email', > + 'web_url', 'scm_url', 'webscm_url') > > > class ProjectViewSet(PatchworkViewSet): > -- > 2.7.4 > > _______________________________________________ > Patchwork mailing list > Patchwork@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork
On 11/19/2016 10:51 AM, Stephen Finucane wrote: > This is apparently the correct way to rename fields, and will ensure a > future API version that supports writes will work correctly. > > Signed-off-by: Stephen Finucane <stephen@that.guru> > Cc: Andy Doan <andy.doan@linaro.org> Much better: Reviewed-by: Andy Doan <andy.doan@linaro.org> > patchwork/api/project.py | 15 +++++++-------- > + # TODO(stephenfin): These should be renamed at the model layer Doesn't seem worth it to me. > + link_name = CharField(max_length=255, source='linkname') > + list_id = CharField(max_length=255, source='listid') > + list_email = CharField(max_length=255, source='listemail')
On Mon, 2016-11-21 at 11:59 +1100, Daniel Axtens wrote: > LGTM! > > The repetition is a bit annoying, but I agree that the old code was > worse. > > I assume that the 255 char limit is what the model? Yup, and one of them was wrong - good spot. Trivial fix in v3. Stephen
diff --git a/patchwork/api/project.py b/patchwork/api/project.py index 17f4b3c..9e5d2aa 100644 --- a/patchwork/api/project.py +++ b/patchwork/api/project.py @@ -17,6 +17,7 @@ # along with Patchwork; if not, write to the Free Software # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +from rest_framework.serializers import CharField from rest_framework.serializers import HyperlinkedModelSerializer from patchwork.api import PatchworkPermission @@ -25,17 +26,15 @@ from patchwork.models import Project class ProjectSerializer(HyperlinkedModelSerializer): - def to_representation(self, instance): - data = super(ProjectSerializer, self).to_representation(instance) - data['link_name'] = data.pop('linkname') - data['list_email'] = data.pop('listemail') - data['list_id'] = data.pop('listid') - return data + # TODO(stephenfin): These should be renamed at the model layer + link_name = CharField(max_length=255, source='linkname') + list_id = CharField(max_length=255, source='listid') + list_email = CharField(max_length=255, source='listemail') class Meta: model = Project - fields = ('url', 'name', 'linkname', 'listid', 'listemail', 'web_url', - 'scm_url', 'webscm_url') + fields = ('url', 'name', 'link_name', 'list_id', 'list_email', + 'web_url', 'scm_url', 'webscm_url') class ProjectViewSet(PatchworkViewSet):
This is apparently the correct way to rename fields, and will ensure a future API version that supports writes will work correctly. Signed-off-by: Stephen Finucane <stephen@that.guru> Cc: Andy Doan <andy.doan@linaro.org> --- patchwork/api/project.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)