Add typing and clean up code in reviewboard.reviews.detail.
Review Request #14309 — Created Jan. 24, 2025 and updated
This introduces type hints all throughout the code that builds the page
data for the review request page. There's a lot of complexity in here
with chances for things to go wrong (such as performance problems, which
this change will help facilitate fixing), so having thorough typing can
help pinpoint possible problems.All attributes for
ReviewRequestPageData
and the entries are now typed
and documented, and all code interfacing with these utilize the right
types. There were a few places where we made some unsafe assumptions,
and these have been addressed.Some of the code has been cleaned up to follow a pattern of building
data structures and then setting them on the class, rather than
re-accessing the data structure multiple times in loops. This is part
optimization, part type safety (we're usingMapping
instead ofdict
,
for instance, to help avoid state pollution), and part preparation for
the round of performance fixes.There are many places where we're doing things we probably shouldn't be
doing. For instance, we set a lot of custom state on models, for use at
entry and render times. Type hints rightly complain about these, so if
we ever decide to revisit some of these operations, they'll be readily
apparent.
All unit tests pass.
Tested against a series of local, heavily-populated review requests.
I didn't notice any regressions.Compared the SQL statements before and after this change using
Django Debug Toolbar. Didn't see any regressions in queries.
Summary | ID |
---|---|
8db9a0f41fb84c8c6c8da790703f6fc9da6a924d |
Description | From | Last Updated | ||
---|---|---|---|---|
We're switching the definition here from defaultdict to list, but then we reassign back to a defaultdict later. Can we … |
|
|||
There are no open issues |