incuna / django-user-management

User management model mixins and api views.
BSD 2-Clause "Simplified" License
57 stars 24 forks source link

Make all fields a tuple in serializers #133

Closed kevinetienne closed 9 years ago

kevinetienne commented 9 years ago

If in a project we define a tuple of core fields and try to concatenate them with the original serializer fields we need to add an exception to RegistrationSerializer.

Making all fields a tuple allow to subclass them more easily.

meshy commented 9 years ago

I also like this, as it stops changes to the fields from accidentally propagating up the tree of subclasses.

kevinetienne commented 9 years ago

Ready for review/merge. Not sure if it was a breaking change but it looks like project sub-classing RegistrationSerializer.Meta.fields might get an error (when concatenated with something other than a tuple).

meshy commented 9 years ago

@KevinEtienne I agree, frustrating as it is, I think this does count as a breaking change. It might be worth expanding on the changelog to briefly explain how to migrate to this version.

kevinetienne commented 9 years ago

Cool thanks, I'll do that

kevinetienne commented 9 years ago

Updated, @Ian-Foote @adam-incuna can you review/merge please?

adam-thomas commented 9 years ago

:+1:

kevinetienne commented 9 years ago

Thanks :)