Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make all fields a tuple in serializers #133

Merged
merged 3 commits into from
Aug 24, 2015

Conversation

kevinetienne
Copy link
Contributor

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.

Kevin Etienne added 2 commits August 21, 2015 12:21
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
Copy link
Contributor

meshy commented Aug 21, 2015

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

@LilyFoote LilyFoote added the bug label Aug 21, 2015
@kevinetienne
Copy link
Contributor Author

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
Copy link
Contributor

meshy commented Aug 21, 2015

@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
Copy link
Contributor Author

Cool thanks, I'll do that

@kevinetienne
Copy link
Contributor Author

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

@adam-thomas
Copy link
Contributor

👍

adam-thomas added a commit that referenced this pull request Aug 24, 2015
@adam-thomas adam-thomas merged commit 3f17917 into master Aug 24, 2015
@adam-thomas adam-thomas deleted the make-field-registration-a-tuple branch August 24, 2015 09:01
@kevinetienne
Copy link
Contributor Author

Thanks :)

class CustomRegistration(RegistrationSerializer):
class Meta(RegistrationSerializer.Meta):
fields = RegistrationSerializer.Meta.fields + ('custom_field',)
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 🍰

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants