Summary

Modernize base resource classes.

Review Request #14256 — Created Dec. 4, 2024 and updated Dec. 12, 2024, 12:22 p.m.

Information

Owner

david

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

This change adds/updates typing and documentation for the base resource
classes.

Testing Done

Ran unit tests.

Commits

Summary	ID
Modernize base resource classes. This change adds/updates typing and documentation for the base resource classes. Testing Done: Ran unit tests.	d5e0de45c634baab7c4e56c0c30207ac09edd4c5

Issues

Description	From	Last Updated
'typing.Type' imported but unused Column: 1 Error code: F401	reviewbot	Dec. 4, 2024, 8:38 a.m.
line too long (80 > 79 characters) Column: 80 Error code: E501	reviewbot	Dec. 4, 2024, 8:38 a.m.
While you're here could you fix up this sentence.	maubin	Dec. 12, 2024, 12:02 p.m.
In RB6+ we have QueryArgs and HTTPHeader type aliases defined in /reviewboard/hostingsvcs/base/http.py. Would it be appropriate to use them here?	maubin	Dec. 12, 2024, 12:03 p.m.
Small nit which I'm not sure even matters but I notice we usually use * instead of - in docstrings.	maubin	Dec. 12, 2024, 12:03 p.m.
Could we use JSONDict instead?	maubin	Dec. 12, 2024, 12:04 p.m.

flake8 failed.

JSHint passed.

flake8

rbtools/api/client.py (Diff revision 1)
The issue has been dropped. Show all issues
```
'typing.Type' imported but unused

Column: 1
Error code: F401
```
rbtools/api/resource/base.py (Diff revision 1)
The issue has been dropped. Show all issues
```
line too long (80 > 79 characters)

Column: 80
Error code: E501
```

Commits:

	Summary	ID
	Modernize base resource classes. This change adds/updates typing and documentation for the base resource classes. Testing Done: Ran unit tests.	b7a94b38bba88ba32623c924b596a6ded24ad8c9
	Modernize base resource classes. This change adds/updates typing and documentation for the base resource classes. Testing Done: Ran unit tests.	4f5819e99c65406fd5eb7130f07e31f08fd55875

Diff:

Revision 2 (+1976 -626)

Show changes

	rbtools/api/client.py
	rbtools/api/factory.py
	rbtools/api/request.py
	rbtools/api/resource/base.py
	rbtools/api/resource/root.py
	rbtools/api/transport/__init__.py
	rbtools/api/transport/sync.py
	rbtools/utils/console.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

rbtools/api/request.py (Diff revision 2)
The issue has been resolved. Show all issues
```
While you're here could you fix up this sentence.
```

rbtools/api/request.py (Diff revision 2)

The issue has been dropped. Show all issues

In RB6+ we have QueryArgs and HTTPHeader type aliases defined in /reviewboard/hostingsvcs/base/http.py. Would it be appropriate to use them here?

david Dec. 12, 2024, 12:06 p.m.

If we used these in more places I'd say yes, but I don't think it's important given the small number of occurrences.

rbtools/api/resource/base.py (Diff revision 2)

The issue has been resolved. Show all issues

Small nit which I'm not sure even matters but I notice we usually use * instead of - in docstrings.

rbtools/api/resource/base.py (Diff revision 2)

The issue has been dropped. Show all issues

Could we use JSONDict instead?

david Dec. 12, 2024, 12:06 p.m.

I don't think JSONDict is entirely accurate here (we don't submit POST requests as JSON, though we probably should allow that). I'm still trying to figure out the best way to do typing for create and update methods generally, so I'd rather keep this as-is for now.

Commits:

	Summary	ID
	Modernize base resource classes. This change adds/updates typing and documentation for the base resource classes. Testing Done: Ran unit tests.	4f5819e99c65406fd5eb7130f07e31f08fd55875
	Modernize base resource classes. This change adds/updates typing and documentation for the base resource classes. Testing Done: Ran unit tests.	d5e0de45c634baab7c4e56c0c30207ac09edd4c5

Diff:

Revision 3 (+1980 -630)

Show changes

	rbtools/api/client.py
	rbtools/api/factory.py
	rbtools/api/request.py
	rbtools/api/resource/base.py
	rbtools/api/resource/root.py
	rbtools/api/transport/__init__.py
	rbtools/api/transport/sync.py
	rbtools/utils/console.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```