-
-
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. |
 david
|
|
|
This could be rewrapped a bit. In particular, it would be nice to have "%s.get_actions" together. |
david
|
|
|
Do you need to modify this test to instantiate ReviewRequestApprovalTestHook now that you've removed that from SandboxExtension? |
david
|
|
|
I think you should remove this and instead instantiate the ReviewRequestApprovalTestHook inside the test_is_approved_sandbox method (like you do for all … |
david
|
ED
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
ReviewRequestApprovalTestHooknow 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
ReviewRequestApprovalTestHookinside thetest_is_approved_sandboxmethod (like you do for all of these new test hooks that you've introduced).