Summary

Clean up the hosting service registry

Review Request #8217 — Created June 3, 2016 and submitted June 17, 2016, 1:32 a.m.

Information

Owner

brennie

Repository

Review Board

Branch

release-3.0.x

Bugs

Depends On

Commit

edb692d...

Reviewers

Groups

reviewboard

People

Description

The hosting services registry was not ported to
djblets.registries.Registry as cleanly as it should have been. As a
result, the "Add repository" form broke, due to missing information on
hosting services. In addition, Review Board 2.0.x introduced a
hosting_service_id attribute which mirrored the id attribute that
3.0.x introduced. We now only use the hosting_service_id attribute.

The "Add repository" form now works correctly and no more errors are
triggered in the log.

Testing Done


Ran unit tests.
Successfully added a repository in the "Add repository" form.

Issues

Description	From	Last Updated
Col: 31 E128 continuation line under-indented for visual indent	reviewbot	June 7, 2016, 4:21 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	June 7, 2016, 4:21 p.m.
The things in the regexes there are safe, but we should escape the hosting service ID (which we weren't doing …	chipx86	June 8, 2016, 2:04 p.m.
Looks like a blank line was added.	chipx86	June 8, 2016, 2:04 p.m.
undefined name 're'	reviewbot	June 8, 2016, 6:24 p.m.
Just to make the tuple super obvious, how about putting the closing paren on the next line? hosting_service_choices.append( (hosting_service.hosting_service_id, hosting_service.name) …	david	June 13, 2016, 5:01 p.m.
Same here.	david	June 13, 2016, 5:02 p.m.
It was this way before, but we should pass in hosting_service.hosting_service_id and e as arguments rather than doing the format …	david	June 17, 2016, 1:31 a.m.

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/hosting_service.py
    reviewboard/webapi/tests/test_hosting_service_account.py
    reviewboard/scmtools/forms.py
    reviewboard/webapi/tests/test_hosting_service.py
    reviewboard/webapi/tests/urls.py
    reviewboard/hostingsvcs/service.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/hosting_service.py
    reviewboard/webapi/tests/test_hosting_service_account.py
    reviewboard/scmtools/forms.py
    reviewboard/webapi/tests/test_hosting_service.py
    reviewboard/webapi/tests/urls.py
    reviewboard/hostingsvcs/service.py

reviewboard/scmtools/forms.py (Diff revision 1)

Show all issues

Col: 31
 E128 continuation line under-indented for visual indent

reviewboard/webapi/resources/hosting_service.py (Diff revision 1)
Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```

Change Summary:

PEP8

Diff:

Revision 2 (+52 -46)

Show changes

	reviewboard/hostingsvcs/service.py
	reviewboard/scmtools/forms.py
	reviewboard/webapi/resources/hosting_service.py
	reviewboard/webapi/tests/test_hosting_service.py
	reviewboard/webapi/tests/test_hosting_service_account.py
	reviewboard/webapi/tests/urls.py

Awesome, it'll be nice to have this fixed :)

reviewboard/hostingsvcs/service.py (Diff revision 2)

Show all issues

The things in the regexes there are safe, but we should escape the hosting service ID (which we weren't doing before), so we can do:

url('^(P<hosting_service_id>%s)/'
    % re.escape(service.hosting_service_id))

reviewboard/scmtools/forms.py (Diff revision 2)
Show all issues
```
Looks like a blank line was added.
```

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/hosting_service.py
    reviewboard/webapi/tests/test_hosting_service_account.py
    reviewboard/scmtools/forms.py
    reviewboard/webapi/tests/test_hosting_service.py
    reviewboard/webapi/tests/urls.py
    reviewboard/hostingsvcs/service.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/hosting_service.py
    reviewboard/webapi/tests/test_hosting_service_account.py
    reviewboard/scmtools/forms.py
    reviewboard/webapi/tests/test_hosting_service.py
    reviewboard/webapi/tests/urls.py
    reviewboard/hostingsvcs/service.py

Change Summary:

Addressed Christian's issues

Diff:

Revision 3 (+50 -46)

Show changes

	reviewboard/hostingsvcs/service.py
	reviewboard/scmtools/forms.py
	reviewboard/webapi/resources/hosting_service.py
	reviewboard/webapi/tests/test_hosting_service.py
	reviewboard/webapi/tests/test_hosting_service_account.py
	reviewboard/webapi/tests/urls.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/hosting_service.py
    reviewboard/webapi/tests/test_hosting_service_account.py
    reviewboard/scmtools/forms.py
    reviewboard/webapi/tests/test_hosting_service.py
    reviewboard/webapi/tests/urls.py
    reviewboard/hostingsvcs/service.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/hosting_service.py
    reviewboard/webapi/tests/test_hosting_service_account.py
    reviewboard/scmtools/forms.py
    reviewboard/webapi/tests/test_hosting_service.py
    reviewboard/webapi/tests/urls.py
    reviewboard/hostingsvcs/service.py

reviewboard/hostingsvcs/service.py (Diff revision 3)
Show all issues
```
 undefined name 're'
```

Change Summary:

add missing import

Diff:

Revision 4 (+51 -46)

Show changes

	reviewboard/hostingsvcs/service.py
	reviewboard/scmtools/forms.py
	reviewboard/webapi/resources/hosting_service.py
	reviewboard/webapi/tests/test_hosting_service.py
	reviewboard/webapi/tests/test_hosting_service_account.py
	reviewboard/webapi/tests/urls.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/hosting_service.py
    reviewboard/webapi/tests/test_hosting_service_account.py
    reviewboard/scmtools/forms.py
    reviewboard/webapi/tests/test_hosting_service.py
    reviewboard/webapi/tests/urls.py
    reviewboard/hostingsvcs/service.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/hosting_service.py
    reviewboard/webapi/tests/test_hosting_service_account.py
    reviewboard/scmtools/forms.py
    reviewboard/webapi/tests/test_hosting_service.py
    reviewboard/webapi/tests/urls.py
    reviewboard/hostingsvcs/service.py

reviewboard/scmtools/forms.py (Diff revision 4)

Show all issues

Just to make the tuple super obvious, how about putting the closing paren on the next line?

hosting_service_choices.append(
    (hosting_service.hosting_service_id, hosting_service.name)
)

reviewboard/scmtools/forms.py (Diff revision 4)
Show all issues
```
Same here.
```

Change Summary:

Addressed open issues

Diff:

Revision 5 (+53 -46)

Show changes

	reviewboard/hostingsvcs/service.py
	reviewboard/scmtools/forms.py
	reviewboard/webapi/resources/hosting_service.py
	reviewboard/webapi/tests/test_hosting_service.py
	reviewboard/webapi/tests/test_hosting_service_account.py
	reviewboard/webapi/tests/urls.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/hosting_service.py
    reviewboard/webapi/tests/test_hosting_service_account.py
    reviewboard/scmtools/forms.py
    reviewboard/webapi/tests/test_hosting_service.py
    reviewboard/webapi/tests/urls.py
    reviewboard/hostingsvcs/service.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/hosting_service.py
    reviewboard/webapi/tests/test_hosting_service_account.py
    reviewboard/scmtools/forms.py
    reviewboard/webapi/tests/test_hosting_service.py
    reviewboard/webapi/tests/urls.py
    reviewboard/hostingsvcs/service.py

Ship it!

reviewboard/scmtools/forms.py (Diff revision 5)

Show all issues

It was this way before, but we should pass in hosting_service.hosting_service_id and e as arguments rather than doing the format ourselves.

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (1b9e6e4)