Add typing and missing documentation to review request fields.

Review Request #14300 — Created Jan. 24, 2025 and updated — Latest diff uploaded

Information

Review Board
release-7.1.x

Reviewers

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.

All unit tests passed.

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

Diff Revision 2

This is not the most recent revision of the diff. The latest diff is revision 3. See what's changed.

orig
1
2
3

Commits

First Last Summary ID Author
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 Christian Hammond
reviewboard/reviews/builtin_fields.py
reviewboard/reviews/fields.py
Loading...