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

remove extraneous about_me serializer fields #2296

Merged
merged 1 commit into from Jan 10, 2017

Conversation

singingwolfboy
Copy link
Contributor

@singingwolfboy singingwolfboy commented Jan 9, 2017

These fields were introduced in #1862, but they aren't actually necessary. Because these classes inherit from ModelSerializer, they automatically generate these fields, so there's no need to specify them (except in the Meta.fields list, where they're already specified).

@odlbot odlbot temporarily deployed to micromasters-ci-pr-2296 January 9, 2017 22:07 Inactive
@codecov-io
Copy link

codecov-io commented Jan 10, 2017

Current coverage is 85.62% (diff: 100%)

Merging #2296 into master will decrease coverage by <.01%

@@             master      #2296   diff @@
==========================================
  Files           238        238          
  Lines         12144      12142     -2   
  Methods        1008       1008          
  Messages          0          0          
  Branches       1640       1640          
==========================================
- Hits          10399      10397     -2   
  Misses         1710       1710          
  Partials         35         35          

Powered by Codecov. Last update fb75ab9...6a6faa7

@giocalitri
Copy link
Contributor

I am not sure I understood the explanation in this PR.

@singingwolfboy
Copy link
Contributor Author

singingwolfboy commented Jan 10, 2017

ProfileSerializer and ProfileFilledOutSerializer both inherit from ModelSerializer, which introspects a Django model class and automatically generates serializer fields based on the model fields. Because of this introspection, there's no need to declare these serializer fields manually, the way this code was doing. All that's necessary is to make sure that about_me is referenced in the fields list, and it already is listed there.

@giocalitri is that clearer?

@giocalitri
Copy link
Contributor

ok, now it is clear: what I understood was that the field was defined in the base class (and I could not find it for obvious reasons)

@giocalitri
Copy link
Contributor

this looks fine to me 👍

@singingwolfboy singingwolfboy merged commit 1a8e811 into master Jan 10, 2017
@singingwolfboy singingwolfboy deleted the remove-extraneous-about-me-serializer-fields branch January 10, 2017 20:22
@gsidebo gsidebo mentioned this pull request Jan 11, 2017
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants