Add typing and fix issues throughout the datagrids code.
Review Request #13355 — Created Oct. 17, 2023 and submitted
The datagrids code was old and crufty, and made some bad typing
assumptions all throughout. A few of these have bit us in the past, when
search indexing bots threw garbage at our datagrids and caused HTTP 500
errors. The mess of types have made it hard to reliably work on this
code, and since it needs some upcoming work, it was time to fix that up.This change adds typing and documentation all throughout the datagrids
code. In the process, bad typing assumptions and scope errors were
fixed.There's a lot of old, bad design patterns in this code, but this doesn't
try to change any of those. It's just building enough of a foundation to
start iterating on.
Djblets and Review Board unit tests pass.
Tested all of the datagrids used in Review Board.
Summary | ID |
---|---|
7f8b1c19d31e94db800032dfa3629404d2edb10e |
Description | From | Last Updated |
---|---|---|
Hold off on this. Some corner cases to resolve. |
chipx86 | |
Missing the type. |
maubin | |
Missing the type. |
maubin | |
While you're here could you fix this typo: "conver" -> "convert". |
maubin | |
We're not doing anything with timezone in this function. I think we're missing a self.timezone = timezone? |
maubin | |
I think the type in the docs is supposed to be dict or or django.template.context.Context. |
maubin | |
Missing documentation for *args and **kwargs. |
maubin |
Change Summary:
Fixed some corner cases due to missing state and some checks for falsiness that should have checked against
None
.
Commits: |
|
|||||||
---|---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+2274 -662) |
Checks run (2 succeeded)
-
-
-
-
djblets/datagrid/grids.py (Diff revision 2) While you're here could you fix this typo: "conver" -> "convert".
-
djblets/datagrid/grids.py (Diff revision 2) We're not doing anything with
timezone
in this function. I think we're missing aself.timezone = timezone
? -
djblets/datagrid/grids.py (Diff revision 2) I think the type in the docs is supposed to be
dict or or django.template.context.Context
. -
Change Summary:
- Fixed some missing types and argument docs, and fixed typos.
- Removed the unused
timezone
argument fromDateTimeSinceColumn
. This was never used, and likely was copy-pasted fromDateTimeColumn
.
Commits: |
|
|||||||
---|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+2294 -666) |