rb_installer scripts in progress

Review Request #2460 — Created July 10, 2011 and discarded

bbasseri
Review Board
reviewboard
The Linux Installer are the older version, The newer version will be posted.
Windows installer are only for the 64bit version OS, but they work with 32bits as well.
Testing was done on 64bit machines.
Description From Last Updated

alphabetical order, and then two linebreaks between the imports and the from import. Two linebreaks between the from imports and …

mike_conleymike_conley

How come only some of these have #! lines at the top?

mike_conleymike_conley

See other comment, re: long strings.

mike_conleymike_conley

space after commas

mike_conleymike_conley

Space after commas

mike_conleymike_conley

Same, re: this directory existing...

mike_conleymike_conley

Are we sure that "C:\Program Files (x86)" will exist?

mike_conleymike_conley

Can we assume that this directory exists?

mike_conleymike_conley

spaces before *

mike_conleymike_conley

ws

mike_conleymike_conley

Spaces on either side of the +

mike_conleymike_conley

spaces on either side of the =

mike_conleymike_conley

Spaces on either side of the =

mike_conleymike_conley

Space before the *.

mike_conleymike_conley

Spaces on either side of the =, +'s

mike_conleymike_conley

Spaces between functions. Applies to all below.

mike_conleymike_conley

Spaces after commas, on either side of =

mike_conleymike_conley

Typo - "installation" - also, why do some of these give off the "installation complete" message, and others do not?

mike_conleymike_conley

Spaces (see above)

mike_conleymike_conley

Spaces for =

mike_conleymike_conley

ws

mike_conleymike_conley

Still need this?

mike_conleymike_conley

Hm - maybe we'll just use standard escaped quotes instead of those weird characters.

mike_conleymike_conley

I see - we build the folder for them...gotcha. Do we clean up afterwards?

mike_conleymike_conley

ws

mike_conleymike_conley

We should probably be consistent with our capitalization.

mike_conleymike_conley

Please put these in alphabetical order

mike_conleymike_conley

Two linebreaks between this from import and the other imports.

mike_conleymike_conley

For these, spaces after the commas. And the PossibleApplications line needs to be broken up over several lines.

mike_conleymike_conley

Spaces on either side of the = sign

mike_conleymike_conley

Spaces on either side of the =

mike_conleymike_conley

Instead of running commands right from the root of the script, I think I'd prefer to have a main function …

mike_conleymike_conley

?

mike_conleymike_conley

ws

mike_conleymike_conley

Temp file

mike_conleymike_conley

temp file

mike_conleymike_conley

temp file

mike_conleymike_conley

temp file

mike_conleymike_conley

Spaces after commas

mike_conleymike_conley

Spaces on either side of =

mike_conleymike_conley

ws

mike_conleymike_conley

ws

mike_conleymike_conley

Needs formatting cleanup, spaces on either side of =... This entire series of instructions, starting from line 26, should probably …

mike_conleymike_conley

I'm seeing lots of "prints" everywhere...shouldn't we be piping to stdout and stderr?

mike_conleymike_conley

ws

mike_conleymike_conley

I think this tilde file is likely temporary, and shouldn't have been included in the review request.

mike_conleymike_conley
BB
mike_conley
  1. Babak:
    
    This is a lot of code!  Holy smokes.  Good hustle.  :)
    
    There are some style and formatting issues that keep cropping up.  I've highlighted a bunch, but you'll really want to run each of these scripts through pep8 before submitting again.
    
    Also, to make things easier on your reviewers - how about breaking up your review requests into the different platforms and architectures, as opposed to one big, monolithic review request?
    
    Thanks!
    
    -Mike
  2. INSTALLER/Linux/Standards/Amazons3storage.py (Diff revision 2)
     
     
     
     
     
     
    alphabetical order, and then two linebreaks between the imports and the from import.  Two linebreaks between the from imports and the first function, and two linebreaks between functions.
    
    These comments apply to all files below.
  3. INSTALLER/Linux/Standards/CVS.py (Diff revision 2)
     
     
    How come only some of these have #! lines at the top?
  4. INSTALLER/Linux/Standards/PostgreSQL.py (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    You might benefit instead with different syntax for long strings - see http://docs.python.org/tutorial/introduction.html#strings
  5. INSTALLER/Linux/Yums/PostgreSQL.py (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    See other comment, re: long strings.
  6. INSTALLER/Linux/Yums/PyLucene.py (Diff revision 2)
     
     
    space after commas
  7. INSTALLER/Linux/Yums/Subversion.py (Diff revision 2)
     
     
    Space after commas
  8. INSTALLER/Windows/X64/Apache-Fastcgi.py (Diff revision 2)
     
     
    Same, re: this directory existing...
  9. INSTALLER/Windows/X64/Apache-Fastcgi.py (Diff revision 2)
     
     
    Are we sure that "C:\Program Files (x86)" will exist?
  10. INSTALLER/Windows/X64/Apache.py (Diff revision 2)
     
     
    Can we assume that this directory exists?
  11. INSTALLER/Windows/X64/CVS.py (Diff revision 2)
     
     
     
    spaces before *
  12. INSTALLER/Windows/X64/CVS.py (Diff revision 2)
     
     
  13. INSTALLER/Windows/X64/CVS.py (Diff revision 2)
     
     
    Spaces on either side of the +
  14. INSTALLER/Windows/X64/Perforce.py (Diff revision 2)
     
     
     
     
    spaces on either side of the =
  15. INSTALLER/Windows/X64/PostgreSQL.py (Diff revision 2)
     
     
     
     
    Spaces on either side of the =
  16. INSTALLER/Windows/X64/Pycrypto.py (Diff revision 2)
     
     
    Space before the *.
  17. INSTALLER/Windows/X64/Pycrypto.py (Diff revision 2)
     
     
     
     
    Spaces on either side of the =, +'s
  18. INSTALLER/Windows/X64/Pycrypto.py (Diff revision 2)
     
     
    Spaces between functions.  Applies to all below.
  19. INSTALLER/Windows/X64/Pycrypto.py (Diff revision 2)
     
     
     
    Spaces after commas, on either side of =
  20. INSTALLER/Windows/X64/Pycrypto.py (Diff revision 2)
     
     
    Typo - "installation" - also, why do some of these give off the "installation complete" message, and others do not?
  21. INSTALLER/Windows/X64/PythonImaginLibrary.py (Diff revision 2)
     
     
     
     
     
    Spaces (see above)
  22. Spaces for =
  23. INSTALLER/Windows/X64/SQLite.py (Diff revision 2)
     
     
     
     
    Still need this?
  24. INSTALLER/Windows/X64/SQLite.py (Diff revision 2)
     
     
     
    Hm - maybe we'll just use standard escaped quotes instead of those weird characters.
  25. INSTALLER/Windows/X64/download.py (Diff revision 2)
     
     
     
    I see - we build the folder for them...gotcha.  Do we clean up afterwards?
  26. INSTALLER/Windows/X64/download.py (Diff revision 2)
     
     
    We should probably be consistent with our capitalization.
  27. INSTALLER/Windows/X64/setpath.py (Diff revision 2)
     
     
    Shorten to <= 80 chars please, and space after commas
  28. INSTALLER/rb-install.py (Diff revision 2)
     
     
     
     
     
     
    Please put these in alphabetical order
  29. INSTALLER/rb-install.py (Diff revision 2)
     
     
    Two linebreaks between this from import and the other imports.
  30. INSTALLER/rb-install.py (Diff revision 2)
     
     
     
     
     
    For these, spaces after the commas.  And the PossibleApplications line needs to be broken up over several lines.
  31. INSTALLER/rb-install.py (Diff revision 2)
     
     
    Spaces on either side of the = sign
  32. INSTALLER/rb-install.py (Diff revision 2)
     
     
     
     
    Spaces on either side of the =
  33. INSTALLER/rb-install.py (Diff revision 2)
     
     
    Instead of running commands right from the root of the script, I think I'd prefer to have a main function that runs each command, and then to call that main function.
  34. INSTALLER/rb-install.py (Diff revision 2)
     
     
  35. gsoc/Linux/Standards/MySQL.py (Diff revision 2)
     
     
  36. gsoc/Linux/Standards/MySQL.py~ (Diff revision 2)
     
     
    Temp file
  37. gsoc/Linux/Standards/PostgreSQL.py~ (Diff revision 2)
     
     
    temp file
  38. gsoc/Linux/Standards/PyLucene.py~ (Diff revision 2)
     
     
    temp file
  39. gsoc/Linux/Standards/SQLite.py~ (Diff revision 2)
     
     
    temp file
  40. gsoc/rb-install.py (Diff revision 2)
     
     
     
     
     
     
     
     
     
    Spaces after commas
  41. gsoc/rb-install.py (Diff revision 2)
     
     
     
     
    Spaces on either side of =
  42. gsoc/rb-install.py (Diff revision 2)
     
     
  43. gsoc/rb-install.py (Diff revision 2)
     
     
  44. gsoc/rb-install.py (Diff revision 2)
     
     
     
    Needs formatting cleanup, spaces on either side of =...
    
    This entire series of instructions, starting from line 26, should probably be in a "main" function, which gets called at the end of the file with sys.argv.
  45. gsoc/rb-install.py (Diff revision 2)
     
     
    I'm seeing lots of "prints" everywhere...shouldn't we be piping to stdout and stderr?
  46. gsoc/rb-install.py (Diff revision 2)
     
     
  47. gsoc/rb-install.py~ (Diff revision 2)
     
     
     
    I think this tilde file is likely temporary, and shouldn't have been included in the review request.
  48. 
      
BB
Review request changed

Status: Discarded

Loading...