Summary

Allow Hosting Service Integration through ReviewBoard Extensions

Review Request #6572 — Created Nov. 9, 2014 and submitted Nov. 21, 2014, 2:13 p.m.

Information

Owner

justy777

Repository

Review Board

Branch

master

Bugs

Depends On

Blocks

6600

Commit

606121d...

Reviewers

Groups

reviewboard, students

People

Description

There's now a HostingServiceHook that extensions can use to register hosting services. This handles registering, unregistering, and retriving the hosting sevices.

Testing Done

Unit test have been written to make sure that HostingService subclasses are registered and unregistered properly.

Issues

Description	From	Last Updated
This test doesn't assert anything, so what is it testing? If you are expecting something to not throw an exception, …	brennie	Nov. 21, 2014, 12:57 p.m.
Like my response about test_register, this test should check that the hosting service is no longer registered.	david	Nov. 21, 2014, 12:57 p.m.

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/hooks.py



Tool: Pyflakes
Processed Files:
    reviewboard/extensions/hooks.py

Summary:

-	[WIP] Allow Hosting Service Integration through ReviewBoard Extensions
+	Allow Hosting Service Integration through ReviewBoard Extensions

Description:

		There's now a SCMToolHook that extensions can use to register source code management tools. This handles registering, unregistering, and retriving the SCM tools.
-
-		TODO:
-		Write unit tests.
-		Make sure the hook is properly intergrated.
-		Write a longer and more informative docsting.

Testing Done:

Unit test have been written to make sure that HostingService subclasses are registered and unregistered properly.

Commit:

-	09dfa100ccac77ff4a46ed46454db0c92c2530bc
+	8265b3f7f2a96337d852de704106b622ed365e35

Diff:

Revision 2 (+52)

Show changes

	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/hooks.py
    reviewboard/extensions/tests.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/hooks.py
    reviewboard/extensions/tests.py

Description:

~		There's now a SCMToolHook that extensions can use to register source code management tools. This handles registering, unregistering, and retriving the SCM tools.
	~	There's now a HostingServiceHook that extensions can use to register hosting services. This handles registering, unregistering, and retriving the hosting sevices.

reviewboard/extensions/tests.py (Diff revision 2)

Show all issues

This test doesn't assert anything, so what is it testing? If you are expecting something to not throw an exception, you should do something like

try:
    something_that_might_raise()
except SomeException as e:
    self.fail("Raised SomeException!")

Same for test_unregister.

justy777 Nov. 19, 2014, 3:02 p.m.

David advised me that it is not necessary. If an exception is thrown then it causes an error, and the test still fails.

david Nov. 21, 2014, 1:49 a.m.

In the case of the sandbox tests, where the purpose of the test is to verify that there are no exceptions, that makes sense. However, this should probably include some assertions that it can find the registered hosting service.

reviewboard/extensions/tests.py (Diff revision 2)

Show all issues

Like my response about test_register, this test should check that the hosting service is no longer registered.

Commit:

-	8265b3f7f2a96337d852de704106b622ed365e35
+	606121dbd4fb3c256de18b99b9a7af146ca64982

Diff:

Revision 3 (+54)

Show changes

	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/hooks.py
    reviewboard/extensions/tests.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/hooks.py
    reviewboard/extensions/tests.py

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-2.0.x (784ad96)