[PATCH v2 03/13] REST: Explicitly define fields

Daniel Axtens dja at axtens.net
Fri Nov 25 11:08:00 AEDT 2016


> This is an artefact from the rebase onto master (where 'fields' had
> already been defined). However, while the 'read_only_fields' change was
> unnecessary (and is reverted in v3), the change to 'field' actually
> makes sense as this affects the ordering of the output. In my mind, the
> context field is of more significant and should come first in the
> output. I can drop this hunk if you disagree, however.

I think the best of both worlds would be to drop the change to
read_only_fields and keep the change to field.

>> These hunks seem to be doing things that are not in the commit
>> message. Eyeballing the diff it seems like you:
>>  - reordered the functions
>>  - renamed some arguments
>>  - added a comment
>>  - made things line up better with DRF idiom.
>
> Mostly the latter - I started using 'SerializerMethodField' instead of
> overriding the 'to_representation' function. I've split this into a
> separate patch for v3.

Many thanks.

Regards,
Daniel


More information about the Patchwork mailing list