Clean up documentation in the HostingService class

Review Request #8913 - Created April 21, 2017 and submitted

Barret Rennie
Review Board
release-2.5.x
8912
8938
reviewboard

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

Built the docs and looked through them.

  • 0
  • 0
  • 27
  • 0
  • 27
Description From Last Updated
Barret Rennie
Christian Hammond
  1. 
      
  2. reviewboard/hostingsvcs/service.py (Diff revision 2)
     
     

    Missing period.

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

    I'd say "The repository the key must be associated with."

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

    We're not adding the key.

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

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

    Same format below.

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

    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.

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

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

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

    Extra blank line.

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

    "The"

    We may want to use a numbered list for these.

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

    "Return"

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

    "Return"

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

    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.

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

    "The provided plan is not valid for the hosting service."

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

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

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

    No indentation.

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

    Same suggestion as earlier.

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

    This is missing a Returns block.

  19. 
      
Barret Rennie
Barret Rennie
David Trowbridge
  1. 
      
  2. reviewboard/hostingsvcs/service.py (Diff revision 4)
     
     
     

    Returns: section?

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

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

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

    Raises?

  5. 
      
Barret Rennie
Review request changed

Change Summary:

Address David's issues

Diff:

Revision 5 (+307 -39)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

Barret Rennie
Barret Rennie
Review request changed

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

Barret Rennie
David Trowbridge
  1. Ship It!
  2. 
      
Barret Rennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.5.x (767821d)
Loading...