Summary

Clean up documentation in the HostingService class

Review Request #8913 — Created April 21, 2017 and submitted May 22, 2017, 10:54 p.m.

Information

Owner

brennie

Repository

Review Board

Branch

release-2.5.x

Bugs

Depends On

~~8912~~

Blocks

8938

Reviewers

Groups

reviewboard

People

Description

This patch brings the rest of hostingsvcs.service into line with our
documentation practices, e.g., module docstrings, PEP 257, etc.

Testing Done

Built the docs and looked through them.

Issues

Description	From	Last Updated
Missing period.	chipx86	April 27, 2017, 3:57 p.m.
I'd say "The repository the key must be associated with."	chipx86	April 27, 2017, 3:58 p.m.
We're not adding the key.	chipx86	April 27, 2017, 3:59 p.m.
These are written more like: "An error occurred during communication with the hosting service." Same format below.	chipx86	April 27, 2017, 3:59 p.m.
These are only ignored in the default implementation. There's much more that's included (and that we should probably document and …	chipx86	April 27, 2017, 4:08 p.m.
I wouldn't say "if applicable." This should instead be elaborated on in the description.	chipx86	April 27, 2017, 4 p.m.
"from"	chipx86	April 27, 2017, 4 p.m.
Extra blank line.	chipx86	April 27, 2017, 4:06 p.m.
"The" We may want to use a numbered list for these.	chipx86	April 27, 2017, 4:01 p.m.
"Return"	chipx86	April 27, 2017, 4:02 p.m.
"Return"	chipx86	April 27, 2017, 4:02 p.m.
Should specify what this means. Otherwise it sounds like it's perhaps a database ID. It's actually specific to the hosting …	chipx86	April 27, 2017, 4:03 p.m.
"The provided plan is not valid for the hosting service."	chipx86	April 27, 2017, 4:04 p.m.
I wouldn't say "in question." Instead, something more like what get_bug_tracker_field has.	chipx86	April 27, 2017, 4:04 p.m.
No indentation.	chipx86	April 27, 2017, 4:04 p.m.
Same suggestion as earlier.	chipx86	April 27, 2017, 4:04 p.m.
This is missing a Returns block.	chipx86	April 27, 2017, 4:05 p.m.
Returns: section?	david	May 11, 2017, 11:10 a.m.
Returns: section? I know that this implementation doesn't return, but it would be nice to list it so subclassers know …	david	May 11, 2017, 11:12 a.m.
Raises?	david	May 11, 2017, 11:12 a.m.
W293 blank line contains whitespace	reviewbot	May 11, 2017, 11:14 a.m.
W293 blank line contains whitespace	reviewbot	May 11, 2017, 11:14 a.m.
W293 blank line contains whitespace	reviewbot	May 11, 2017, 11:14 a.m.
W293 blank line contains whitespace	reviewbot	May 11, 2017, 11:14 a.m.
W293 blank line contains whitespace	reviewbot	May 11, 2017, 11:14 a.m.
W293 blank line contains whitespace	reviewbot	May 11, 2017, 11:14 a.m.
W293 blank line contains whitespace	reviewbot	May 11, 2017, 12:19 p.m.

JSHint passed.

PEP8 Style Checker passed.

Pyflakes passed.

Diff:

Revision 2 (+279 -34)

Show changes

reviewboard/hostingsvcs/service.py

Checks run (2 succeeded, 1 failed with error)

JSHint passed.

PEP8 Style Checker internal error.

Pyflakes passed.

reviewboard/hostingsvcs/service.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Missing period.
```
reviewboard/hostingsvcs/service.py (Diff revision 2)
The issue has been resolved. Show all issues
```
I'd say "The repository the key must be associated with."
```
reviewboard/hostingsvcs/service.py (Diff revision 2)
The issue has been resolved. Show all issues
```
We're not adding the key.
```

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

The issue has been resolved. Show all issues

These are written more like: "An error occurred during communication with the hosting service."

Same format below.

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

The issue has been resolved. Show all issues

These are only ignored in the default implementation. There's much more that's included (and that we should probably document and provide in the signature).

See the call to hosting_service.check_repository in reviewboard/scmtools/forms.py.

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

The issue has been resolved. Show all issues

I wouldn't say "if applicable." This should instead be elaborated on in the description.

reviewboard/hostingsvcs/service.py (Diff revision 2)
The issue has been resolved. Show all issues
```
"from"
```
reviewboard/hostingsvcs/service.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Extra blank line.
```
reviewboard/hostingsvcs/service.py (Diff revision 2)
The issue has been resolved. Show all issues
```
"The"
We may want to use a numbered list for these.
```
reviewboard/hostingsvcs/service.py (Diff revision 2)
The issue has been resolved. Show all issues
```
"Return"
```
reviewboard/hostingsvcs/service.py (Diff revision 2)
The issue has been resolved. Show all issues
```
"Return"
```

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

The issue has been resolved. Show all issues

Should specify what this means. Otherwise it sounds like it's perhaps a database ID. It's actually specific to the hosting service in question, and should be opaque to the caller.

reviewboard/hostingsvcs/service.py (Diff revision 2)
The issue has been resolved. Show all issues
```
"The provided plan is not valid for the hosting service."
```

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

The issue has been resolved. Show all issues

I wouldn't say "in question." Instead, something more like what get_bug_tracker_field has.

reviewboard/hostingsvcs/service.py (Diff revision 2)
The issue has been resolved. Show all issues
```
No indentation.
```
reviewboard/hostingsvcs/service.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Same suggestion as earlier.
```
reviewboard/hostingsvcs/service.py (Diff revision 2)
The issue has been resolved. Show all issues
```
This is missing a Returns block.
```

Change Summary:

Addressed Christian's issues

Diff:

Revision 3 (+292 -39)

Show changes

reviewboard/hostingsvcs/service.py

Checks run (3 succeeded)

JSHint passed.

PEP8 Style Checker passed.

Pyflakes passed.

Change Summary:

Correct return type of get_change

Diff:

Revision 4 (+289 -39)

Show changes

reviewboard/hostingsvcs/service.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/hostingsvcs/service.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Returns: section?
```

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

The issue has been resolved. Show all issues

Returns: section? I know that this implementation doesn't return, but it would be nice to list it so subclassers know what to do.

reviewboard/hostingsvcs/service.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Raises?
```

Change Summary:

Address David's issues

Diff:

Revision 5 (+307 -39)

Show changes

reviewboard/hostingsvcs/service.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/hostingsvcs/service.py (Diff revision 5)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/hostingsvcs/service.py (Diff revision 5)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/hostingsvcs/service.py (Diff revision 5)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/hostingsvcs/service.py (Diff revision 5)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/hostingsvcs/service.py (Diff revision 5)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/hostingsvcs/service.py (Diff revision 5)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```