Refactor the actions system
Review Request #9131 — Created Aug. 11, 2017 and discarded
This is a large patch that refactors the action registry system
implemented by a past student to use a proper registry. Much refactoring
has been done:
- Action base classes now live in
reviewboard.actions. This package
also includes header dropdown actions. - Review request-specific action classes now live in
reviewboard.reviews.actions. - ALL action hooks support new-style action classes and old-style
dicts. Previously, there was a discrepancy betweeen which hooks
supported which type of action. ActionHookis now deprecated in favour ofReviewRequestActionHook.
The behaviour was the previously the same, except ActionHook supported
old-style dicts instead of classes. Now that the behaviour of all
hooks has been unified, we no longer need this distinction.- The
{% child_actions %}templatetag has been moved fromreviewtags
to a newactionstemplate tag library. It now takes the action it
should list the child actions of. - The
_DictActionand_DictMenuActionclasses have been turned into
theDictActionMixin. - There are now two action registries: one for review request actions
and one for header actions. This allows us to keep them seperate. - We now store instances of dict actions instead of the dicts
themselves. This is because you cannot store adictin aset(as
it is unhashable) and therefore cannot be put into aRegistry. - Generic action templates now live in
templates/actions.
- Ran unit tests.
- Built the docs and looked through them.
| Description | From | Last Updated |
|---|---|---|
|
W391 blank line at end of file |
|
|
|
W391 blank line at end of file |
|
|
|
F401 'reviewboard.actions.header.HeaderMenuAction' imported but unused |
|
|
|
F401 'reviewboard.actions.header.HeaderAction' imported but unused |
|
|
|
F841 local variable 'hook' is assigned to but never used |
|
|
|
F821 undefined name 'ReviewRequestAction' |
|
|
|
F821 undefined name 'ReviewRequestMenuAction' |
|
|
|
Should be changed to 4.0 now. |
|
|
|
Here, too. |
|
|
|
And here. |
|
|
|
= should be removed. |
|
|
|
Blank line between these. |
|
|
|
Capitalize the start of "subclasses." |
|
|
|
Only 1 blank line. |
|
|
|
Here, too. |
|
|
|
Here, too. |
|
|
|
Here, too. |
|
|
|
Missing docs. |
|
|
|
Alphabetical order. |
|
|
|
Missing docs. |
|
|
|
Missing docs. |
|
|
|
Missing a unicode_literals import. |
|
|
|
Missing docs. |
|
|
|
Missing the rest of the sentence. Also need one for image_height separately. |
|
|
|
Missing docs. |
|
|
|
Missing docs. |
|
|
|
Missing unicode_literals. |
|
|
|
Incomplete docstring. |
|
|
|
Missing period. |
|
|
|
Must be the full class path. |
|
|
|
Shouldn't use the "If ..." form. Instead, "The parent action is not registered." |
|
|
|
Same here. |
|
|
|
No trailing comma on the last parameter to a method. |
|
|
|
Missing docs. |
|
|
|
Leftover debugging. |
|
|
|
Missing docs. |
|
|
|
Must have the full class path. |
|
|
|
Just "The item was not foudn in the registry." |
|
|
|
Full class path. |
|
|
|
Full class path. |
|
|
|
Alignment issue. |
|
|
|
Same notes as above with the descriptions. |
|
|
|
Full class path. |
|
|
|
Description on the next line. |
|
|
|
We should have a module-level logger. |
|
|
|
Missing a file docstring. |
|
|
|
Missing a unicode_literals import. |
|
|
|
Needs a full class path. |
|
|
|
Description on the next line. |
|
|
|
Alphabetical order. |
|
|
|
Should be 4.0 now. And actually, this can now look like: """ ... Deprecated: 3.0: Blah blah """ |
|
|
|
Missing docs. |
|
|
|
Should use str() instead of b'' for future Python compatibility. |
|
|
|
Needs the full class path. |
|
|
|
Same here. |
|
|
|
And here. |
|
|
|
And here. |
|
|
|
Double repr. We don't need the explicit call. |
|
|
|
No double underscore on the function. That will mangle the name. Needs docs. |
|
|
|
Needs a full class path. |
|
|
|
Missing docs. |
|
|
|
Should use str() instead of b''. |
|
|
|
Needs a full class path. |
|
|
|
Needs a full class path. |
|
|
|
Needs a full class path. |
|
|
|
These can be on the same line. |
|
|
|
No double underscore. Needs docs. |
|
|
|
Leftover debugging. |
|
|
|
Missing a test docstring. |
|
|
|
Missing a test docstring. |
|
|
|
Missing a test docstring. |
|
|
|
Missing a test docstring. |
|
|
|
Missing a test docstring. |
|
|
|
Missing a test docstring. |
|
|
|
Missing a test docstring. |
|
|
|
Missing a test docstring. |
|
|
|
Missing a test docstring. |
|
|
|
Swap these. |
|
|
|
Missing docs. |
|
|
|
Missing docs. |
|
|
|
Missing docs. |
|
|
|
Missing docs. |
|
|
|
Missing docs. |
|
|
|
Missing docs. |
|
|
|
Missing docs. |
|
|
|
Missing docs. |
|
|
|
Missing docs. |
|
|
|
No need for staticfiles or i18n. |
|
|
|
There should be a book for this. |
|
|
|
E702 multiple statements on one line (semicolon) |
|
- Description:
-
This is a large patch that refactors the action registry system
implemented by a past student to use a proper registry. Much refactoring has been done: - Action base classes now live in
reviewboard.actions. This package
also includes header dropdown actions.
- Review request-specific action classes now live in
reviewboard.reviews.actions.
- ALL action hooks support new-style action classes and old-style
dicts. Previously, there was a discrepancy betweeen which hooks
supported which type of action.
ActionHookis now deprecated in favour ofReviewRequestActionHook.
The behaviour was the previously the same, except ActionHook supported
old-style dicts instead of classes. Now that the behaviour of all
hooks has been unified, we no longer need this distinction.
- The
{% child_actions %}templatetag has been moved fromreviewtags
to a newactionstemplate tag library. It now takes the action it
should list the child actions of.
- The
_DictActionand_DictMenuActionclasses have been turned into
theDictActionMixin.
- There are now two action registries: one for review request actions
and one for header actions. This allows us to keep them seperate.
- We now store instances of dict actions instead of the dicts
themselves. This is because you cannot store adictin aset(as
it is unhashable) and therefore cannot be put into aRegistry.
- Generic action templates now live in
templates/actions.
~ TODO: Docs
~ TODO: more unit tests.
- Action base classes now live in
- Testing Done:
-
~ Ran unit tests.
~ TODO: Smoke testing, more unit tests. ~ - Ran unit tests.
~ - Built the docs and looked through them.
- Commit:
-
add0aa18159f99869655273a52f40cbf98cda8c90ccb1bf8908fccba01c1cd83f801be0df14215da
- Diff:
-
Revision 2 (+1256 -1428)
- Added Files:
Checks run (2 succeeded)
- Change Summary:
-
Rebase
- Commit:
-
0ccb1bf8908fccba01c1cd83f801be0df14215da373472950257d0eee0be15cf79493d74a4137af3
- Diff:
-
Revision 3 (+1247 -1414)
- Change Summary:
-
Fix typo
- Commit:
-
373472950257d0eee0be15cf79493d74a4137af3a809dfe28650e474822990ae5adc13a5422f71b0
- Diff:
-
Revision 4 (+1247 -1414)
Checks run (2 succeeded)
- Change Summary:
-
Fix templates
- Commit:
-
a809dfe28650e474822990ae5adc13a5422f71b0a10e69fd25ab2807d1b0b3d8f8e8dc7d151eade7
- Diff:
-
Revision 5 (+1249 -1421)
Checks run (2 succeeded)
- Summary:
-
WIP: Refactor the actions systemRefactor the actions system
- Description:
-
This is a large patch that refactors the action registry system
implemented by a past student to use a proper registry. Much refactoring has been done: - Action base classes now live in
reviewboard.actions. This package
also includes header dropdown actions.
- Review request-specific action classes now live in
reviewboard.reviews.actions.
- ALL action hooks support new-style action classes and old-style
dicts. Previously, there was a discrepancy betweeen which hooks
supported which type of action.
ActionHookis now deprecated in favour ofReviewRequestActionHook.
The behaviour was the previously the same, except ActionHook supported
old-style dicts instead of classes. Now that the behaviour of all
hooks has been unified, we no longer need this distinction.
- The
{% child_actions %}templatetag has been moved fromreviewtags
to a newactionstemplate tag library. It now takes the action it
should list the child actions of.
- The
_DictActionand_DictMenuActionclasses have been turned into
theDictActionMixin.
- There are now two action registries: one for review request actions
and one for header actions. This allows us to keep them seperate.
- We now store instances of dict actions instead of the dicts
themselves. This is because you cannot store adictin aset(as
it is unhashable) and therefore cannot be put into aRegistry.
- Generic action templates now live in
templates/actions.
- - TODO: more unit tests.
- Action base classes now live in
-
This is an old change at this point, and I'm in no way expecting you to finish this work. I just wanted to go through it and make suitable notes on what needs to be done before inclusion, so that we're at a good point to hand it off and continue work.
Of course you're always more than welcome to finish the change, but that's not the priority right now :)
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
- Change Summary:
-
This patch can now be applied to release-3.0.x to ease forward porting to 4.0.x.
- Commit:
-
a10e69fd25ab2807d1b0b3d8f8e8dc7d151eade7cc0e50c126f85f58d819bb5f2a2f144a03b3f749
- Diff:
-
Revision 6 (+1354 -1409)