Summary

Add typing and missing documentation to review request fields.

Review Request #14300 — Created Jan. 24, 2025 and updated 3 days, 9 hours ago

Information

Owner

chipx86*

Repository

Review Board

Branch

release-7.1.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description*

The review request fields codebase is pretty crucial to Review Board,

and one of the primary extension interfaces that companies tend to

use and specialize.
In an effort to modernize fields and help reason about them in other

code that needs to interact with them, this change adds typing

throughout the fields codebases.
Fields are now generics, storing the type of data that can be

represented when loading and saving values.
All rendering functions now have return types of SafeString.
The base functions rendering or serializing change description state

work with Any as the type. This is because, while we have a

somewhat-standard format most fields use to record state in

ChangeDescription.fields_changed, this is entirely at the whim of the

implementation. As such, it's up to the field class to narrow the type

as needed and perform any checks and parsing required.

Testing Done

All unit tests passed.

Tested all the field types on a review request, and a wide variety of
change description renderings.

Commits

Summary	ID
Add typing and missing documentation to review request fields. The review request fields codebase is pretty crucial to Review Board, and one of the primary extension interfaces that companies tend to use and specialize. In an effort to modernize fields and help reason about them in other code that needs to interact with them, this change adds typing throughout the fields codebases. Fields are now generics, storing the type of data that can be represented when loading and saving values. All rendering functions now have return types of `SafeString`. The base functions rendering or serializing change description state work with `Any` as the type. This is because, while we have a somewhat-standard format most fields use to record state in `ChangeDescription.fields_changed`, this is entirely at the whim of the implementation. As such, it's up to the field class to narrow the type as needed and perform any checks and parsing required. In the process, several typing issues, bad assumptions, and incorrect documentation were found and fixed.	61ee33db0bfe3bc3afbab334122e9edb6a2db3b9

Summary

Add typing and missing documentation to review request fields.

The review request fields codebase is pretty crucial to Review Board, and one of the primary extension interfaces that companies tend to use and specialize. In an effort to modernize fields and help reason about them in other code that needs to interact with them, this change adds typing throughout the fields codebases. Fields are now generics, storing the type of data that can be represented when loading and saving values. All rendering functions now have return types of `SafeString`. The base functions rendering or serializing change description state work with `Any` as the type. This is because, while we have a somewhat-standard format most fields use to record state in `ChangeDescription.fields_changed`, this is entirely at the whim of the implementation. As such, it's up to the field class to narrow the type as needed and perform any checks and parsing required. In the process, several typing issues, bad assumptions, and incorrect documentation were found and fixed.

61ee33db0bfe3bc3afbab334122e9edb6a2db3b9

Issues

Description	From	Last Updated
Could say the User type here instead of object. And now that we've switched to importing from reviewboard.accounts.models.User, is that …	maubin	3 days, 19 hours ago
Group instead of object.	maubin	3 days, 19 hours ago
User instead of object.	maubin	3 days, 19 hours ago
You missed a super() here.	david	3 days, 19 hours ago
Forgot to add the type here.	maubin	3 days, 19 hours ago
And here.	maubin	3 days, 19 hours ago
Missing types on some of the attributes here.	maubin	3 days, 19 hours ago
'djblets.util.typing.JSONDict' imported but unused Column: 5 Error code: F401	reviewbot	3 days, 19 hours ago
There are no open issues

JSHint passed.

Ship it!

reviewboard/reviews/fields.py (Diff revision 1)
The issue has been resolved. Show all issues
```
You missed a super() here.
```

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

The issue has been resolved. Show all issues

Could say the User type here instead of object. And now that we've switched to importing from reviewboard.accounts.models.User, is that the value that we should use moving forward in docstrings instead of django.contrib.auth.models.User?

reviewboard/reviews/builtin_fields.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Group instead of object.
```
reviewboard/reviews/builtin_fields.py (Diff revision 1)
The issue has been resolved. Show all issues
```
User instead of object.
```
reviewboard/reviews/fields.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Forgot to add the type here.
```
reviewboard/reviews/fields.py (Diff revision 1)
The issue has been resolved. Show all issues
```
And here.
```
reviewboard/reviews/fields.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Missing types on some of the attributes here.
```
1. chipx86 3 days, 19 hours ago
  These are setting values defined in the base class.

Change Summary:


Modernized another super() call.
Fixed docstrings to reference specific object types where possible.
Added typing to some default_value attributes when introduced in a class.

Commits:

	Summary	ID
	Add typing and missing documentation to review request fields. The review request fields codebase is pretty crucial to Review Board, and one of the primary extension interfaces that companies tend to use and specialize. In an effort to modernize fields and help reason about them in other code that needs to interact with them, this change adds typing throughout the fields codebases. Fields are now generics, storing the type of data that can be represented when loading and saving values. All rendering functions now have return types of `SafeString`. The base functions rendering or serializing change description state work with `Any` as the type. This is because, while we have a somewhat-standard format most fields use to record state in `ChangeDescription.fields_changed`, this is entirely at the whim of the implementation. As such, it's up to the field class to narrow the type as needed and perform any checks and parsing required. In the process, several typing issues, bad assumptions, and incorrect documentation were found and fixed.	03186ee8fef188fc54d74ee130fb1a55ac448dfb
	Add typing and missing documentation to review request fields. The review request fields codebase is pretty crucial to Review Board, and one of the primary extension interfaces that companies tend to use and specialize. In an effort to modernize fields and help reason about them in other code that needs to interact with them, this change adds typing throughout the fields codebases. Fields are now generics, storing the type of data that can be represented when loading and saving values. All rendering functions now have return types of `SafeString`. The base functions rendering or serializing change description state work with `Any` as the type. This is because, while we have a somewhat-standard format most fields use to record state in `ChangeDescription.fields_changed`, this is entirely at the whim of the implementation. As such, it's up to the field class to narrow the type as needed and perform any checks and parsing required. In the process, several typing issues, bad assumptions, and incorrect documentation were found and fixed.	e29c78d9189ea424f0be403220261e50480762d6

Diff:

Revision 2 (+1818 -686)

Show changes

	reviewboard/reviews/builtin_fields.py
	reviewboard/reviews/fields.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/reviews/fields.py (Diff revision 2)
The issue has been resolved. Show all issues
```
'djblets.util.typing.JSONDict' imported but unused

Column: 5
Error code: F401
```

Change Summary:

Removed an unused import.

Commits:

	Summary	ID
	Add typing and missing documentation to review request fields. The review request fields codebase is pretty crucial to Review Board, and one of the primary extension interfaces that companies tend to use and specialize. In an effort to modernize fields and help reason about them in other code that needs to interact with them, this change adds typing throughout the fields codebases. Fields are now generics, storing the type of data that can be represented when loading and saving values. All rendering functions now have return types of `SafeString`. The base functions rendering or serializing change description state work with `Any` as the type. This is because, while we have a somewhat-standard format most fields use to record state in `ChangeDescription.fields_changed`, this is entirely at the whim of the implementation. As such, it's up to the field class to narrow the type as needed and perform any checks and parsing required. In the process, several typing issues, bad assumptions, and incorrect documentation were found and fixed.	e29c78d9189ea424f0be403220261e50480762d6
	Add typing and missing documentation to review request fields. The review request fields codebase is pretty crucial to Review Board, and one of the primary extension interfaces that companies tend to use and specialize. In an effort to modernize fields and help reason about them in other code that needs to interact with them, this change adds typing throughout the fields codebases. Fields are now generics, storing the type of data that can be represented when loading and saving values. All rendering functions now have return types of `SafeString`. The base functions rendering or serializing change description state work with `Any` as the type. This is because, while we have a somewhat-standard format most fields use to record state in `ChangeDescription.fields_changed`, this is entirely at the whim of the implementation. As such, it's up to the field class to narrow the type as needed and perform any checks and parsing required. In the process, several typing issues, bad assumptions, and incorrect documentation were found and fixed.	61ee33db0bfe3bc3afbab334122e9edb6a2db3b9

Diff:

Revision 3 (+1818 -686)

Show changes

	reviewboard/reviews/builtin_fields.py
	reviewboard/reviews/fields.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```