Summary

Flesh out the documentation for ReviewRequestManager query functions.

Review Request #12340 — Created June 8, 2022 and submitted June 16, 2022, 7:08 p.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-5.0.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

The query functions in ReviewRequestManager are very old, and most
either lacked documentation or didn't meet current documentation
conventions.

As some work is being done in these functions, and given the importance
of these functions and their behavior, it's time to ensure they meet
documentation standards.

This updates the docstrings for all the query functions, making
behaviors and usage explicit.

No logic has changed.

Testing Done

Built the docs and checked through them. Didn't see anything
obvious or any build errors/warnings related to this file.

Commits

Summary	ID
Flesh out the documentation for ReviewRequestManager query functions. The query functions in `ReviewRequestManager` are very old, and most either lacked documentation or didn't meet current documentation conventions. As some work is being done in these functions, and given the importance of these functions and their behavior, it's time to ensure they meet documentation standards. This updates the docstrings for all the query functions, making behaviors and usage explicit. No logic has changed.	6cceba85a82c9c7c993c047240239e398f0a55f1

Summary

Flesh out the documentation for ReviewRequestManager query functions.

The query functions in `ReviewRequestManager` are very old, and most either lacked documentation or didn't meet current documentation conventions. As some work is being done in these functions, and given the importance of these functions and their behavior, it's time to ensure they meet documentation standards. This updates the docstrings for all the query functions, making behaviors and usage explicit. No logic has changed.

6cceba85a82c9c7c993c047240239e398f0a55f1

Issues

Description	From	Last Updated
Could shorten this to just "The User instance or username."	maubin	June 10, 2022, 7:09 p.m.
Change to "The User instance or username."	maubin	June 10, 2022, 7:09 p.m.
Can change this to "The query object." and move the information about the query into the description	maubin	June 10, 2022, 7:10 p.m.
Should we mention that if a local site is given, it is assumed that the user (if given) has access …	maubin	June 10, 2022, 7:10 p.m.
Add a Raises: section for the django.contrib.auth.models.User.DoesNotExist error that could happen in in self.get_to_or_from_user_query	maubin	June 10, 2022, 7:11 p.m.

flake8 passed.

JSHint passed.

reviewboard/reviews/managers.py (Diff revision 1)

The issue has been dropped. Show all issues

Could shorten this to just "The User instance or username."

chipx86 June 10, 2022, 7:14 p.m.

Think I actually need to elaborate more. In the queries, there's a couple references to users. The viewing user and the owner. I'm going to change these to be explicit about what's expected where.

reviewboard/reviews/managers.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Change to "The User instance or username."
```
1. chipx86 June 10, 2022, 7:14 p.m.
  There's an example of one that differs in meaning from the previous.

reviewboard/reviews/managers.py (Diff revision 1)

The issue has been resolved. Show all issues

Can change this to "The query object." and move the information about the query into the description

reviewboard/reviews/managers.py (Diff revision 1)

The issue has been resolved. Show all issues

Should we mention that if a local site is given, it is assumed that the user (if given) has access to that local site (i.e. we don't check whether a user has access to that local site). If so we could include that here under the arg description (and every where else where a local_site arg is used) or maybe under the description for the method where you talk about filtering based on access

reviewboard/reviews/managers.py (Diff revision 1)

The issue has been resolved. Show all issues

Add a Raises: section for the django.contrib.auth.models.User.DoesNotExist error that could happen in in self.get_to_or_from_user_query

Change Summary:


Clarified each user-related argument.
Simplified some redundant information in Returns.
Added a missing Raises.
Updated local_site arguments to mention that the caller should first validate user access.

Commits:

	Summary	ID
	Flesh out the documentation for ReviewRequestManager query functions. The query functions in `ReviewRequestManager` are very old, and most either lacked documentation or didn't meet current documentation conventions. As some work is being done in these functions, and given the importance of these functions and their behavior, it's time to ensure they meet documentation standards. This updates the docstrings for all the query functions, making behaviors and usage explicit. No logic has changed.	dfdfe536b669660289e3ad297ca7e2851caacce0
	Flesh out the documentation for ReviewRequestManager query functions. The query functions in `ReviewRequestManager` are very old, and most either lacked documentation or didn't meet current documentation conventions. As some work is being done in these functions, and given the importance of these functions and their behavior, it's time to ensure they meet documentation standards. This updates the docstrings for all the query functions, making behaviors and usage explicit. No logic has changed.	6cceba85a82c9c7c993c047240239e398f0a55f1

Diff:

Revision 2 (+614 -56)

Show changes

reviewboard/reviews/managers.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

Good call on being more explicit about the user args

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-5.0.x (bbb7b44)