-
Notifications
You must be signed in to change notification settings - Fork 81
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
Improve documentation for users app #1292
Conversation
…nd this command is run via cron
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.
Thanks for your work updating documentation, this will be very helpful for me and other new team members.
I had one minor nit, but feel free to ignore.
Returns | ||
---------- | ||
Queryset | ||
A queryset of user objects |
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.
minor nit: QuerySet instead of Queryset for consistency.
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.
Thanks for all of your work adding more documentation in key places in the library. I'll go ahead and merge this once I commit the suggestions below 😄
Updated and expanded documentation for the
users
app. This PR also includes some minor tidying of unused variables.Where changes were more than documentation changes or might have been confusing I made individual commits to aid with review.
Phabricator Ticket
T264425
How Has This Been Tested?
Tests passed, and did some clicking around locally. I tested the Authorization admin page because I was especially unsure about that code removal.
Types of changes
What types of changes does your code introduce? Add an
x
in all the boxes that apply: