david got a fish trophy!

Summary

Split up rbtools.api.resource.

Review Request #14241 — Created Nov. 11, 2024 and submitted Feb. 7, 2025, 8:19 a.m.

Information

Owner

david

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

The resource definitions for RBTools are quite lengthy. While the bulk
of the current implementation all kind of needs to stick together, we
can split out individual resource-specific subclasses.

This change does not make any functional or other cleanup changes to the
code, just splits it up.

Testing Done

Ran unit tests.

Commits

Summary	ID
Split up rbtools.api.resource. The resource definitions for RBTools are quite lengthy. While the bulk of the current implementation all kind of needs to stick together, we can split out individual resource-specific subclasses. The bulk of this is just splitting up code without making any other cleanup or functional changes. The only exception is DraftDiffResource, which was created for a mimetype that doesn't exist in Review Board (the server-side DraftDiffResource reports its mimetype as `diff` instead of `draft-diff`. This resource subclass has been removed and the old name is aliased to DiffResource. Testing Done: Ran unit tests.	4d0ca0d1eeef5997bb5e90d7cf79f7f2196f9e98

Summary

Split up rbtools.api.resource.

The resource definitions for RBTools are quite lengthy. While the bulk of the current implementation all kind of needs to stick together, we can split out individual resource-specific subclasses. The bulk of this is just splitting up code without making any other cleanup or functional changes. The only exception is DraftDiffResource, which was created for a mimetype that doesn't exist in Review Board (the server-side DraftDiffResource reports its mimetype as `diff` instead of `draft-diff`. This resource subclass has been removed and the old name is aliased to DiffResource. Testing Done: Ran unit tests.

4d0ca0d1eeef5997bb5e90d7cf79f7f2196f9e98

Issues

Description	From	Last Updated
undefined name 'HttpRequest' Column: 19 Error code: F821	reviewbot	Nov. 11, 2024, 1:43 p.m.
blank line at end of file Column: 1 Error code: W391	reviewbot	Nov. 11, 2024, 1:43 p.m.
I know there's benefits to this syntax but I kind of don't like using it. It makes it a lot …	chipx86	Jan. 23, 2025, 4:19 p.m.
The role name for these is :py:mod:. This comment applies to all the new modules.	chipx86	Jan. 23, 2025, 4:26 p.m.

flake8 failed.

JSHint passed.

flake8

rbtools/api/resource/draft_diff_commit.py (Diff revision 1)
The issue has been dropped. Show all issues
```
undefined name 'HttpRequest'

Column: 19
Error code: F821
```
rbtools/api/resource/root.py (Diff revision 1)
The issue has been dropped. Show all issues
```
blank line at end of file

Column: 1
Error code: W391
```

Commits:

	Summary	ID
	Split up rbtools.api.resource. The resource definitions for RBTools are quite lengthy. While the bulk of the current implementation all kind of needs to stick together, we can split out individual resource-specific subclasses. This change does not make any functional or other cleanup changes to the code, just splits it up. Testing Done: Ran unit tests.	176a23a6f6688259b434deb2f1e832bd52121037
	Split up rbtools.api.resource. The resource definitions for RBTools are quite lengthy. While the bulk of the current implementation all kind of needs to stick together, we can split out individual resource-specific subclasses. This change does not make any functional or other cleanup changes to the code, just splits it up. Testing Done: Ran unit tests.	d7fb51dd07f8f940e383163bb9b891354865df26

Diff:

Revision 2 (+1918 -1362)

Show changes

	rbtools/api/resource/__init__.py
	rbtools/api/resource.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
	10 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

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

The issue has been resolved. Show all issues

I know there's benefits to this syntax but I kind of don't like using it. It makes it a lot harder to search the codebase for certain imports (which I've done a lot while working on projects) and to order imports alongside non-relative imports.
Ages ago, I remember this topic coming up with contributed patches and we decided the benefits of the full class name outweighed the shorter line lengths. I think that's still the right call.

rbtools/api/resource/diff_file_attachment.py (Diff revision 2)
The issue has been resolved. Show all issues
```
The role name for these is :py:mod:. This comment applies to all the new modules.
```

Commits:

	Summary	ID
	Split up rbtools.api.resource. The resource definitions for RBTools are quite lengthy. While the bulk of the current implementation all kind of needs to stick together, we can split out individual resource-specific subclasses. This change does not make any functional or other cleanup changes to the code, just splits it up. Testing Done: Ran unit tests.	d7fb51dd07f8f940e383163bb9b891354865df26
	Split up rbtools.api.resource. The resource definitions for RBTools are quite lengthy. While the bulk of the current implementation all kind of needs to stick together, we can split out individual resource-specific subclasses. The bulk of this is just splitting up code without making any other cleanup or functional changes. The only exception is DraftDiffResource, which was created for a mimetype that doesn't exist in Review Board (the server-side DraftDiffResource reports its mimetype as `diff` instead of `draft-diff`. This resource subclass has been removed and the old name is aliased to DiffResource. Testing Done: Ran unit tests.	4d0ca0d1eeef5997bb5e90d7cf79f7f2196f9e98

Diff:

Revision 3 (+1884 -1362)

Show changes

	rbtools/api/resource/__init__.py
	rbtools/api/resource.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
	9 more

Checks run (1 timed out)

JSHint timed out.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (5a6bd07)