[WIP] HostingServices: add implementation of get_remote_repositories for Beanstalk

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

olessia
Review Board
master
reviewboard, students
chipx86

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)
     
     

    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)
     
     

    No space before the """

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

    This can probably just be one line.

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

    Blank line before for loops.

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

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

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

    type is a reserved word.

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

    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)
     
     
     

    Blank lines around if statements.

  9. 
      
OL
Review request changed

Status: Discarded

Loading...