probably should check for ftp also ?

You don't need the parentheses here. I'd also like it if it was explicit about the different prefixes:

if origin_url.startswith('http://') or origin_url.startswith('https://'):

You don't need ftp, because as far as I know, there's no git ftp transport :)

No parentheses around the if conditions here.

wrong return comment.

Works for me against a 1.5 RC1 server.

There needs to be a space added at the end of the first line.

Single _, and no trailing __.

Space before "url"

The first line should always be a one-line summary.

Then a blank line, followed by any human-readable documentation.

The information about Python versions would belong as comments, not in the documentation. However, we absolutely must support Python 2.4, so you should use the old version of the results from urlparse.

A more appropriate name may be "url_schemas".

Blank line before the commets.

Comments should always be in sentence casing, and must have a space between the "#" and text.

Blank line between these.

This is sort of weird. Please do:

for url_schema in url_schemas:
    if url.startswith(url_schema):
        ...

Can be combined to one line.

Parameters must always have a space between the preceding comma and the parameter. So, url.replace(url, http)

Space around operators.

pr.username + '@', '', 1

Blank line between these.

Space around =

Blank line between these.

Space around =

So, I'd actually rather not go this route of having the option, even if it's configurable. It means people have to do a lot more manual work to get going.

Instead, it'd be much better for Review Board to try two versions of the URL. One with the username, one without. There are other SCMClients (such as the Perforce one) that do a variety of checks for repository names.

Only special operator functions built-in to Python can have the __foo__ naming convention. This should be _get_urls.

No need for the \

The previous statement should have just returned this instead of storing in a variable first.

Unit test docstrings must always be only one line that briefly describes the unit test. Same line as the """. This is what we'll show when verbosely showing the unit tests that are running.

Same comments about the comment.

Blank line between these.