-
Notifications
You must be signed in to change notification settings - Fork 167
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
fixed failed to load api specification error #363
fixed failed to load api specification error #363
Conversation
@Somya-Singhal Thanks 🙏 We'll review this shortly! |
else: | ||
return CreatorSerializer | ||
|
||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring below should be a part of the docstring at the beginning of the class, not here
hello @Somya-Singhal, thanks for creating this PR! I left a comment on your changes. Once that is fixed, this PR will be merged. |
Also, improvements can be made to your PR description. Try to make the description describe exactly what you did. |
Ok sure @NdibeRaymond, I will make changes shortly and update you. |
Hello @NdibeRaymond @tuxology, I have made the changes as requested. Please do let me know if any further changes are required. |
|
||
Requires username of user. | ||
Returns user profile. | ||
Note that this schema returns the full user profile, but the api sometimes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring should be added to the previous docstring, not replace it. It should look more like
"""
Fetch Profile of user with given username.
Requires username of user.
Returns user profile.
Note that this schema returns the full user profile, but the api sometimes
returns a minimal version of the user profile, omitting certain fields that
are not neccessary or are sensitive.
"""
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NdibeRaymond, I have fixed it. Now could you please check?
Great! Something I do most times (and I think makes sense in this case ) is to always rebase multiple commits in a single PR into a single commit before merging, especially for a minor change like this one. Other than that, we are good to merge. |
Sure @NdibeRaymond, I will make sure to do it in my next PR's. Thanks for the information. |
fixed failed to load api specification error
fixed failed to load api specification error (unstructuredstudio#363) fixed failed to load api specification error added style to fix text wrap (unstructuredstudio#373) Add hot reloading to media container (unstructuredstudio#382) Add new search features (unstructuredstudio#362) * Finish creators and projects search migration * Finish creators and projects search migration * Add tag search * Add search for projects with tags * Fix search bar on mobile * use 0 rather than 0 with units in css * Remove print * Default search type to projects * Disable scroll lock Revert package-lock (unstructuredstudio#383) improveConsistency: fixed title text small, extra vertical space ... (unstructuredstudio#369) * improveConsistency: fixed title text small, extra vertical space and inconsitent padding of buttons fix text overflow issue in signup page phone number field (unstructuredstudio#378) removed spacing between icons
Closes #357
Changes
UserProfileAPIView
classget_serializer_class
method.UserProfileAPIView
class docstring.