david got a fish trophy!

Summary

Add new resources, part 2/7.

Review Request #14341 — Created Feb. 10, 2025 and updated March 20, 2025, 9:47 a.m.

Information

Owner

david

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

This change adds the following new resource implementations:
- ChangeItemResource
- ChangeListResource
- DiffContextResource
- LastUpdateResource
- ReviewRequestDraftResource
- StatusUpdateItemResource
- StatusUpdateListResource

Testing Done


Ran unit tests.
Used the new resources from a test script and saw that everything

  worked as expected.

Commits

Summary	ID
Add new resources, part 2/7. This change adds the following new resource implementations: - ChangeItemResource - ChangeListResource - DiffContextResource - LastUpdateResource - ReviewRequestDraftResource - StatusUpdateItemResource - StatusUpdateListResource Testing Done: - Ran unit tests. - Used the new resources from a test script and saw that everything worked as expected.	6e0ee6adeb52a440e4587d34c03874c665daed4f

Issues

Description	From	Last Updated
Something that occurred to me while reviewing this. Since we have some deprecated functions and some new ones, we might …	chipx86	March 20, 2025, 9:47 a.m.
line too long (80 > 79 characters) Column: 80 Error code: E501	reviewbot	Feb. 10, 2025, 9:54 a.m.
This isn't needed in the TypedDict.	chipx86	March 20, 2025, 9:42 a.m.
Can we link this here and below? We can use standard Sphinx references, since we reference RB docs.	chipx86	March 20, 2025, 9:43 a.m.
The API doesn't allow for mutating this, so we should make this a Mapping.	chipx86	March 20, 2025, 9:43 a.m.
This is in a TypedDict, so we don't need this here.	chipx86	March 20, 2025, 9:44 a.m.
These are all Mapping now.	chipx86	March 20, 2025, 9:44 a.m.
Small nit, but can we list these as bullet points?	chipx86	March 20, 2025, 9:45 a.m.
Can we separate these out into paragraph per, so it doesn't all run together? Alternatively, worth having an enum? This …	chipx86	March 20, 2025, 9:45 a.m.
Should we mark this deprecated then? Even if we're going to keep it around for backwards-compatibility.	chipx86	March 20, 2025, 9:46 a.m.
Same question as above.	chipx86	March 20, 2025, 9:47 a.m.
And here. And others that are considered legacy.	chipx86	March 20, 2025, 9:47 a.m.

flake8 failed.

JSHint passed.

flake8

rbtools/api/resource/base_review_request.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
	Add new resources, part 2/7. This change adds the following new resource implementations: - ChangeItemResource - ChangeListResource - DiffContextResource - LastUpdateResource - ReviewRequestDraftResource - StatusUpdateItemResource - StatusUpdateListResource Testing Done: - Ran unit tests. - Used the new resources from a test script and saw that everything worked as expected.	6f5602a17810328abd46baddf518db01559a6bea
	Add new resources, part 2/7. This change adds the following new resource implementations: - ChangeItemResource - ChangeListResource - DiffContextResource - LastUpdateResource - ReviewRequestDraftResource - StatusUpdateItemResource - StatusUpdateListResource Testing Done: - Ran unit tests. - Used the new resources from a test script and saw that everything worked as expected.	728914f6dedfac55afb384a5ad57b3561bfa2ade

Diff:

Revision 2 (+2368 -136)

Show changes

	rbtools/api/resource/__init__.py
	rbtools/api/resource/base_review_request.py
	rbtools/api/resource/change.py
	rbtools/api/resource/diff.py
	rbtools/api/resource/diff_context.py
	rbtools/api/resource/file_diff.py
	rbtools/api/resource/last_update.py
	rbtools/api/resource/review_request.py
	3 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Commits:

	Summary	ID
	Add new resources, part 2/7. This change adds the following new resource implementations: - ChangeItemResource - ChangeListResource - DiffContextResource - LastUpdateResource - ReviewRequestDraftResource - StatusUpdateItemResource - StatusUpdateListResource Testing Done: - Ran unit tests. - Used the new resources from a test script and saw that everything worked as expected.	728914f6dedfac55afb384a5ad57b3561bfa2ade
	Add new resources, part 2/7. This change adds the following new resource implementations: - ChangeItemResource - ChangeListResource - DiffContextResource - LastUpdateResource - ReviewRequestDraftResource - StatusUpdateItemResource - StatusUpdateListResource Testing Done: - Ran unit tests. - Used the new resources from a test script and saw that everything worked as expected.	b4920d5c38f97eaf38c9760cc881c9b3ff89822d

Diff:

Revision 3 (+2376 -140)

Show changes

	rbtools/api/resource/__init__.py
	rbtools/api/resource/base_review_request.py
	rbtools/api/resource/change.py
	rbtools/api/resource/diff.py
	rbtools/api/resource/diff_context.py
	rbtools/api/resource/last_update.py
	rbtools/api/resource/review_request.py
	rbtools/api/resource/review_request_draft.py
	2 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

The issue has been dropped. Show all issues

Something that occurred to me while reviewing this. Since we have some deprecated functions and some new ones, we might want to include for every function a Review Board compatibility range of some sort. This absolutely doesn't need to be part of this change, but it's probably worth having, especially as we introduce new APIs.

david March 20, 2025, 9:47 a.m.
```
Will think about it.
```

rbtools/api/resource/change.py (Diff revision 3)
The issue has been resolved. Show all issues
```
This isn't needed in the TypedDict.
```

rbtools/api/resource/change.py (Diff revision 3)

The issue has been resolved. Show all issues

Can we link this here and below? We can use standard Sphinx references, since we reference RB docs.

rbtools/api/resource/change.py (Diff revision 3)
The issue has been resolved. Show all issues
```
The API doesn't allow for mutating this, so we should make this a Mapping.
```
rbtools/api/resource/diff_context.py (Diff revision 3)
The issue has been resolved. Show all issues
```
This is in a TypedDict, so we don't need this here.
```
rbtools/api/resource/diff_context.py (Diff revision 3)
The issue has been resolved. Show all issues
```
These are all Mapping now.
```
rbtools/api/resource/last_update.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Small nit, but can we list these as bullet points?
```

rbtools/api/resource/last_update.py (Diff revision 3)

The issue has been resolved. Show all issues

Can we separate these out into paragraph per, so it doesn't all run together?

Alternatively, worth having an enum? This question more broadly applies to other Literal instances. Purely food for thought, and nothing that would need to go into this set of changes.

rbtools/api/resource/root.py (Diff revision 3)

The issue has been dropped. Show all issues

Should we mark this deprecated then? Even if we're going to keep it around for backwards-compatibility.

david March 20, 2025, 9:47 a.m.

Let me think about your point above regarding versioning, since I think anything we do there would impact this.

rbtools/api/resource/root.py (Diff revision 3)
The issue has been dropped. Show all issues
```
Same question as above.
```
rbtools/api/resource/root.py (Diff revision 3)
The issue has been dropped. Show all issues
```
And here. And others that are considered legacy.
```

Commits:

	Summary	ID
	Add new resources, part 2/7. This change adds the following new resource implementations: - ChangeItemResource - ChangeListResource - DiffContextResource - LastUpdateResource - ReviewRequestDraftResource - StatusUpdateItemResource - StatusUpdateListResource Testing Done: - Ran unit tests. - Used the new resources from a test script and saw that everything worked as expected.	b4920d5c38f97eaf38c9760cc881c9b3ff89822d
	Add new resources, part 2/7. This change adds the following new resource implementations: - ChangeItemResource - ChangeListResource - DiffContextResource - LastUpdateResource - ReviewRequestDraftResource - StatusUpdateItemResource - StatusUpdateListResource Testing Done: - Ran unit tests. - Used the new resources from a test script and saw that everything worked as expected.	6e0ee6adeb52a440e4587d34c03874c665daed4f

Diff:

Revision 4 (+2384 -140)

Show changes

	rbtools/api/resource/__init__.py
	rbtools/api/resource/base_review_request.py
	rbtools/api/resource/change.py
	rbtools/api/resource/diff.py
	rbtools/api/resource/diff_context.py
	rbtools/api/resource/last_update.py
	rbtools/api/resource/review_request.py
	rbtools/api/resource/review_request_draft.py
	2 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.