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.

Sign up for GitHub

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

Jump to bottom

moved expandable_fields to meta class #6198

Merged
merged 3 commits into from
Oct 1, 2019
Merged

moved expandable_fields to meta class #6198

merged 3 commits into from
Oct 1, 2019

Conversation

Iamshankhadeep
Copy link
Contributor

fixes #6159

@Iamshankhadeep
Copy link
Contributor Author

can anyone tell me why lint is failing?

saadmk11
saadmk11 reviewed Sep 20, 2019
View reviewed changes
Copy link
Member

@saadmk11 saadmk11 left a comment

Choose a reason for hiding this comment

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

This should fix the lint issue :)

readthedocs/api/v3/serializers.py Outdated Show resolved Hide resolved
readthedocs/api/v3/serializers.py Outdated Show resolved Hide resolved
@Iamshankhadeep
Copy link
Contributor Author

lint is failing because of the space and newline issues? @saadmk11

@saadmk11
Copy link
Member

I think so. You can see the error log from here https://travis-ci.org/readthedocs/readthedocs.org/jobs/587389577

@Iamshankhadeep Iamshankhadeep force-pushed the expandable_fields_to_meta branch from 2b8bbc1 to 6402c26 Compare September 20, 2019 13:35
stsewd
stsewd reviewed Sep 23, 2019
View reviewed changes
Copy link
Member

@stsewd stsewd left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, but you are only refactoring the expandable_fields attribute. It should be moved to the Meta class.

@Iamshankhadeep
Copy link
Contributor Author

Sorry I do not know what is meta class can you provide me with some link that can help me to learn 'meta class'. Thanks.

@stsewd
Copy link
Member

stsewd commented Sep 24, 2019

@Iamshankhadeep you should check the docs for django drf flex fields https://github.com/rsinger86/drf-flex-fields

stsewd
stsewd approved these changes Sep 26, 2019
View reviewed changes
Copy link
Member

@stsewd stsewd left a comment

Choose a reason for hiding this comment

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

Thanks!

@Iamshankhadeep
Copy link
Contributor Author

@stsewd dont I have to rebase this pull request?

@stsewd
Copy link
Member

stsewd commented Sep 27, 2019

@Iamshankhadeep no, it's fine.

@Iamshankhadeep
Copy link
Contributor Author

okay @stsewd, Sir thank you a lot for helping me and @saadmk11 thanks for helping me understanding the lint error.

@stsewd stsewd merged commit 0b2e1d5 into readthedocs:master Oct 1, 2019
@Iamshankhadeep Iamshankhadeep deleted the expandable_fields_to_meta branch October 1, 2019 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saadmk11 saadmk11 saadmk11 left review comments

@stsewd stsewd stsewd approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Move expandable_fields to meta class
3 participants
@Iamshankhadeep @saadmk11 @stsewd