[WIP] HostingServices: add implementation of get_remote_repositories for Beanstalk

Review Request #5469 — Created Feb. 13, 2014 and discarded

Information

Review Board
master

Reviewers

HostingServices: add implementation of get_remote_repositories for Beanstalk

Manual testing

Description From Last Updated

Looking at _api_get, it looks like it will always return something, unless it raises an Exception. So are we waiting …

mike_conleymike_conley

No space before the """

chipx86chipx86

This can probably just be one line.

chipx86chipx86

Blank line before for loops.

chipx86chipx86

type is a reserved word. For consistency, how about scm_type?

chipx86chipx86

type is a reserved word.

chipx86chipx86

We try to keep the statements simpler. Can you have a separate statement for setting mirror_path to None? Or just …

chipx86chipx86

Blank lines around if statements.

chipx86chipx86
mike_conley
  1. Looking pretty solid so far - just one concern - see below.

  2. reviewboard/hostingsvcs/beanstalk.py (Diff revision 1)
     
     
    Show all issues

    Looking at _api_get, it looks like it will always return something, unless it raises an Exception.

    So are we waiting for response to be the empty string here as our exit condition?

    If that's the case, that sounds fragile (it makes us vulnerable to changes in Beanstalk's API that might put us in an infinite loop, if I'm understanding this correctly).

  3. 
      
OL
OL
chipx86
  1. 
      
  2. reviewboard/hostingsvcs/beanstalk.py (Diff revision 2)
     
     
    Show all issues

    No space before the """

  3. reviewboard/hostingsvcs/beanstalk.py (Diff revision 2)
     
     
     
    Show all issues

    This can probably just be one line.

  4. reviewboard/hostingsvcs/beanstalk.py (Diff revision 2)
     
     
    Show all issues

    Blank line before for loops.

  5. reviewboard/hostingsvcs/beanstalk.py (Diff revision 2)
     
     
    Show all issues

    type is a reserved word. For consistency, how about scm_type?

  6. reviewboard/hostingsvcs/beanstalk.py (Diff revision 2)
     
     
    Show all issues

    type is a reserved word.

  7. reviewboard/hostingsvcs/beanstalk.py (Diff revision 2)
     
     
    Show all issues

    We try to keep the statements simpler. Can you have a separate statement for setting mirror_path to None?

    Or just set it below when yielding, actually.

  8. reviewboard/hostingsvcs/beanstalk.py (Diff revision 2)
     
     
     
    Show all issues

    Blank lines around if statements.

  9. 
      
OL
Review request changed

Status: Discarded

Loading...