> 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
_______________________________________________
Patchwork mailing list
[email protected]
https://lists.ozlabs.org/listinfo/patchwork

Reply via email to