Summary

Add typing and modernize docs for resource subclasses.

Review Request #14245 — Created Nov. 13, 2024 and submitted Feb. 7, 2025, 4:19 p.m.

Information

Owner

david

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

This change modernizes the existing resource subclasses, fixing up
typing and documentation. This also adds any missing item or list
resource subclasses to match the ones we have.

Testing Done


Ran unit tests.
Used the RBTools API for various tests.

Commits

Summary	ID
Add typing and modernize docs for resource subclasses. This change modernizes the existing resource subclasses, fixing up typing and documentation. This also adds any missing item or list resource subclasses to match the ones we have. Testing Done: - Ran unit tests. - Used the RBTools API for various tests.	31d7b46a1d08bd1b09b460e988f3d5f9c0406dfa

Issues

Description	From	Last Updated
multiple spaces after keyword Column: 36 Error code: E271	reviewbot	Nov. 13, 2024, 5:20 p.m.
missing whitespace around operator Column: 6 Error code: E225	reviewbot	Nov. 13, 2024, 5:20 p.m.
Swap these (ordering issue).	chipx86	Jan. 24, 2025, 12:52 a.m.
While cleaning up these, it might be a good time to move to keyword-only arguments and use housekeeping to deprecate …	chipx86	Jan. 24, 2025, 12:54 a.m.
These should probably say dict of ....	chipx86	Jan. 24, 2025, 12:55 a.m.
Another good candidate, if it's worth moving to kwonly args in this change.	chipx86	Jan. 24, 2025, 12:55 a.m.
These are in a different order from the argument list.	chipx86	Jan. 24, 2025, 12:55 a.m.
Another candidate.	chipx86	Jan. 24, 2025, 12:55 a.m.

flake8 failed.

JSHint passed.

flake8

rbtools/api/resource/draft_diff_commit.py (Diff revision 1)
The issue has been dropped. Show all issues
```
multiple spaces after keyword

Column: 36
Error code: E271
```
rbtools/api/resource/review_request.py (Diff revision 1)
The issue has been dropped. Show all issues
```
missing whitespace around operator

Column: 6
Error code: E225
```

Commits:

	Summary	ID
	Add typing and modernize docs for resource subclasses. This change modernizes the existing resource subclasses, fixing up typing and documentation. This also adds any missing item or list resource subclasses to match the ones we have.	6d95d09c215e540e170f60d6b693ce4bc5996e5b
	Add typing and modernize docs for resource subclasses. This change modernizes the existing resource subclasses, fixing up typing and documentation. This also adds any missing item or list resource subclasses to match the ones we have.	3529401517bee7583803a6045f5bb861a38a85b0

Diff:

Revision 2 (+1094 -276)

Show changes

	pyproject.toml
	rbtools/api/resource/__init__.py
	rbtools/api/resource/diff.py
	rbtools/api/resource/diff_commit.py
	rbtools/api/resource/diff_file_attachment.py
	rbtools/api/resource/draft_diff.py
	rbtools/api/resource/draft_diff_commit.py
	rbtools/api/resource/draft_file_attachment.py
	9 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Very nice.

A few suggestions and couple example places for those suggestions, if you want to make that part of this change, but otherwise this pretty much looks good to go to me.

pyproject.toml (Diff revision 2)
The issue has been resolved. Show all issues
```
Swap these (ordering issue).
```

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

The issue has been dropped. Show all issues

While cleaning up these, it might be a good time to move to keyword-only arguments and use housekeeping to deprecate the positionals. Doesn't have to be in this change (I don't want to expand the scope), so feel free to drop.

david Jan. 24, 2025, 1:13 a.m.

Definitely, but something for later. I'll make a note.

rbtools/api/resource/diff.py (Diff revision 2)
The issue has been resolved. Show all issues
```
These should probably say dict of ....
```
rbtools/api/resource/draft_diff_commit.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Another good candidate, if it's worth moving to kwonly args in this change.
```

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

The issue has been resolved. Show all issues

These are in a different order from the argument list.

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

The issue has been dropped. Show all issues

Another candidate.

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

Just an observation and room for possible future improvement. We type the return of HttpRequest, but document a resource type. The latter is what the caller should ultimately expect but the former is the reality of the function. Not sure what to do there. Should HttpRequest someday be generic with the resulting payload type? Something we can mull over.

david Jan. 24, 2025, 1:13 a.m.

This all improves significantly in later changes with @request_method_returns[...]().

Commits:

	Summary	ID
	Add typing and modernize docs for resource subclasses. This change modernizes the existing resource subclasses, fixing up typing and documentation. This also adds any missing item or list resource subclasses to match the ones we have.	3529401517bee7583803a6045f5bb861a38a85b0
	Add typing and modernize docs for resource subclasses. This change modernizes the existing resource subclasses, fixing up typing and documentation. This also adds any missing item or list resource subclasses to match the ones we have. Testing Done: - Ran unit tests. - Used the RBTools API for various tests.	31d7b46a1d08bd1b09b460e988f3d5f9c0406dfa

Diff:

Revision 3 (+1100 -272)

Show changes

	pyproject.toml
	rbtools/api/resource/__init__.py
	rbtools/api/resource/diff.py
	rbtools/api/resource/diff_commit.py
	rbtools/api/resource/diff_file_attachment.py
	rbtools/api/resource/draft_diff_commit.py
	rbtools/api/resource/draft_file_attachment.py
	rbtools/api/resource/draft_screenshot.py
	8 more

Checks run (1 timed out)

JSHint timed out.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (4651576)