-
-
reviewboard/extensions/templatetags/rb_extensions.py (Diff revision 1) Pleane remove the space at the end of this string.
Adding sandboxing to action_hooks
Review Request #5710 — Created April 13, 2014 and submitted
Information | |
---|---|
edwinzg | |
Review Board | |
master | |
Reviewers | |
reviewboard, students | |
Added sandboxing to action_hooks
Added unit tests to test suites at reviewboard/extensions/tests.py
Unit testing done - tested all 5 different action hooks (DiffViewerActionHook, HeaderActionHook, HeaderDropdownActionHook, ReivewRequestActionHook, ReviewRequestDropdownActionHook) with and without the sandboxing
Ran on all tests in extensions/tests.py
Description | From | Last Updated |
---|---|---|
Pleane remove the space at the end of this string. |
|
|
This could be rewrapped a bit. In particular, it would be nice to have "%s.get_actions" together. |
|
|
Do you need to modify this test to instantiate ReviewRequestApprovalTestHook now that you've removed that from SandboxExtension? |
|
|
I think you should remove this and instead instantiate the ReviewRequestApprovalTestHook inside the test_is_approved_sandbox method (like you do for all … |
|
Summary: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
|||||||||
Testing Done: |
|
|||||||||
Diff: |
Revision 2 (+131 -8) |
-
This patch no longer applies cleanly. Can you rebase this onto the latest release-2.0.x and update?
-
reviewboard/extensions/templatetags/rb_extensions.py (Diff revision 2) This could be rewrapped a bit. In particular, it would be nice to have "%s.get_actions" together.
-
-
reviewboard/extensions/tests.py (Diff revision 3) Do you need to modify this test to instantiate
ReviewRequestApprovalTestHook
now that you've removed that fromSandboxExtension
? -
Can you run all the unit tests, not just the newly-added ones?
-
-
reviewboard/extensions/tests.py (Diff revision 4) I think you should remove this and instead instantiate the
ReviewRequestApprovalTestHook
inside thetest_is_approved_sandbox
method (like you do for all of these new test hooks that you've introduced).