Summary

Modernized function typing in reviewboard/accounts/models.py

Review Request #14106 — Created Aug. 16, 2024 and submitted Sept. 11, 2024, 3:59 p.m.

Information

Owner

dan.casares

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Modernized function typing in reviewboard/accounts/models.py

Testing Done

No new tests were added, however the entire reviewboard test suite
was run and all previously passing tests still pass.

Commits

Summary	ID	Author
Modernized function typing in reviewboard/accounts/models.py Added typing to all the function parameters and return types in every function to bring the file in line with more modern typing standards. Certain type imports were put under a `TYPE_CHECKING`. No new tests were added, however the entire `reviewboard` test suite was run and all previously passing tests still pass.	3e36873cc624c53798cfa815d192edeb9ecea6f2	Daniel

Issues

Description	From	Last Updated
Below, you changed models.Model to be Model, but you didn't change the imports here.	david	Sept. 9, 2024, 10:53 a.m.
Let's add a "Returns:" section to the comment too.	david	Sept. 9, 2024, 10:53 a.m.
This needs a trailing comma. Same for many others in this change.	david	Sept. 9, 2024, 10:53 a.m.
Let's add "Returns:" here too.	david	Sept. 9, 2024, 10:53 a.m.
This should be typed as Group	david	Sept. 9, 2024, 10:53 a.m.
'xhrUnknownErrorText' is defined but never used. Column: 7 Error code: W098	reviewbot	Sept. 9, 2024, 11:03 a.m.
While we usually treat long imports like this, for typing imports we try to fit as many as them on …	maubin	Sept. 9, 2024, 4:53 p.m.
The closing """ should be on its own line.	maubin	Sept. 9, 2024, 4:53 p.m.
While you're here, can you add a Type: bool after the description in the docstring.	maubin	Sept. 9, 2024, 4:53 p.m.
The closing """ should be on its own line.	maubin	Sept. 9, 2024, 4:53 p.m.
While these types are correct, we actually don't use type annotations for args and *kwargs, so you can remove the …	maubin	Sept. 9, 2024, 4:53 p.m.
Same comment here for **kwargs, we can remove the type.	maubin	Sept. 9, 2024, 4:53 p.m.

flake8 passed.

JSHint passed.

Commits:

	Summary	ID	Author
	Modernized function typing in reviewboard/accounts/models.py Added typing to all the function parameters and return types in every function to bring the file in line with more modern typing standards. Certain type imports were put under a `TYPE_CHECKING`. No new tests were added, however the entire `reviewboard` test suite was run and all previously passing tests still pass.	5f47f776d27ef8971a0cfbd5c20142bd356c2724	Daniel
	Modernized function typing in reviewboard/accounts/models.py Added typing to all the function parameters and return types in every function to bring the file in line with more modern typing standards. Certain type imports were put under a `TYPE_CHECKING`. No new tests were added, however the entire `reviewboard` test suite was run and all previously passing tests still pass.	0149e77fc2277cd24c0c5745e3b32f6c6134e5cc	Daniel

Diff:

Revision 2 (+286 -90)

Show changes

reviewboard/accounts/models.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/accounts/models.py (Diff revision 2)

The issue has been resolved. Show all issues

Below, you changed models.Model to be Model, but you didn't change the imports here.

reviewboard/accounts/models.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Let's add a "Returns:" section to the comment too.
```
reviewboard/accounts/models.py (Diff revision 2)
The issue has been resolved. Show all issues
```
This needs a trailing comma. Same for many others in this change.
```
reviewboard/accounts/models.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Let's add "Returns:" here too.
```
reviewboard/accounts/models.py (Diff revision 2)
The issue has been resolved. Show all issues
```
This should be typed as Group
```

Commits:

	Summary	ID	Author
	Modernized function typing in reviewboard/accounts/models.py Added typing to all the function parameters and return types in every function to bring the file in line with more modern typing standards. Certain type imports were put under a `TYPE_CHECKING`. No new tests were added, however the entire `reviewboard` test suite was run and all previously passing tests still pass.	0149e77fc2277cd24c0c5745e3b32f6c6134e5cc	Daniel
	Modernized function typing in reviewboard/accounts/models.py Added typing to all the function parameters and return types in every function to bring the file in line with more modern typing standards. Certain type imports were put under a `TYPE_CHECKING`. No new tests were added, however the entire `reviewboard` test suite was run and all previously passing tests still pass.	d72bc111c58e20f2fa7a9a78eac84e1d0f19e14a	Daniel

Diff:

Revision 3 (+3842 -5907)

Show changes

	dev-requirements.txt
	contrib/docker/Dockerfile
	docs/manual/extending/coderef/index.rst
	docs/releasenotes/7.0.2.rst
	docs/releasenotes/index.rst
	docs/releasenotes/_static/images/7.x/7.0.2-admin-draft-view.png
	reviewboard/dependencies.py
	reviewboard/staticbundles.py
	120 more

Checks run (1 failed, 1 succeeded)

flake8 passed.

JSHint failed.

JSHint

reviewboard/static/rb/js/accountPrefsPage/views/oauthTokensView.es6.js (Diff revision 3)
The issue has been dropped. Show all issues
```
'xhrUnknownErrorText' is defined but never used.

Column: 7
Error code: W098
```

Commits:

	Summary	ID	Author
	Modernized function typing in reviewboard/accounts/models.py Added typing to all the function parameters and return types in every function to bring the file in line with more modern typing standards. Certain type imports were put under a `TYPE_CHECKING`. No new tests were added, however the entire `reviewboard` test suite was run and all previously passing tests still pass.	d72bc111c58e20f2fa7a9a78eac84e1d0f19e14a	Daniel
	Modernized function typing in reviewboard/accounts/models.py Added typing to all the function parameters and return types in every function to bring the file in line with more modern typing standards. Certain type imports were put under a `TYPE_CHECKING`. No new tests were added, however the entire `reviewboard` test suite was run and all previously passing tests still pass.	86371cd3e2f0740f6ea2747d28409eaaf059475a	Daniel

Diff:

Revision 4 (+304 -86)

Show changes

reviewboard/accounts/models.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

```
Thanks for modernizing this.
```

reviewboard/accounts/models.py (Diff revision 4)

The issue has been resolved. Show all issues

While we usually treat long imports like this, for typing imports we try to fit as many as them on one line as possible because its very common for us to import a lot of things from typing. So this should look like this instead:
from typing import (Any, ClassVar, Dict, List, Optional, Set, Tuple,
                    TYPE_CHECKING, Union)

reviewboard/accounts/models.py (Diff revision 4)
The issue has been resolved. Show all issues
```
The closing """ should be on its own line.
```

reviewboard/accounts/models.py (Diff revision 4)

The issue has been resolved. Show all issues

While you're here, can you add a

Type:
    bool

after the description in the docstring.

reviewboard/accounts/models.py (Diff revision 4)
The issue has been resolved. Show all issues
```
The closing """ should be on its own line.
```

reviewboard/accounts/models.py (Diff revision 4)

The issue has been resolved. Show all issues

While these types are correct, we actually don't use type annotations for *args and **kwargs, so you can remove the types here. I'm not really sure why we don't type them.

reviewboard/accounts/models.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Same comment here for **kwargs, we can remove the type.
```

Commits:

	Summary	ID	Author
	Modernized function typing in reviewboard/accounts/models.py Added typing to all the function parameters and return types in every function to bring the file in line with more modern typing standards. Certain type imports were put under a `TYPE_CHECKING`. No new tests were added, however the entire `reviewboard` test suite was run and all previously passing tests still pass.	86371cd3e2f0740f6ea2747d28409eaaf059475a	Daniel
	Modernized function typing in reviewboard/accounts/models.py Added typing to all the function parameters and return types in every function to bring the file in line with more modern typing standards. Certain type imports were put under a `TYPE_CHECKING`. No new tests were added, however the entire `reviewboard` test suite was run and all previously passing tests still pass.	3e36873cc624c53798cfa815d192edeb9ecea6f2	Daniel

Diff:

Revision 5 (+300 -86)

Show changes

reviewboard/accounts/models.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed