rb_installer scripts in progress

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

Information

Review Board

Reviewers

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)
     
     
     
     
     
     
    Show all issues
    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)
     
     
    Show all issues
    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)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    Show all issues
    See other comment, re: long strings.
  6. INSTALLER/Linux/Yums/PyLucene.py (Diff revision 2)
     
     
    Show all issues
    space after commas
  7. INSTALLER/Linux/Yums/Subversion.py (Diff revision 2)
     
     
    Show all issues
    Space after commas
  8. INSTALLER/Windows/X64/Apache-Fastcgi.py (Diff revision 2)
     
     
    Show all issues
    Same, re: this directory existing...
  9. INSTALLER/Windows/X64/Apache-Fastcgi.py (Diff revision 2)
     
     
    Show all issues
    Are we sure that "C:\Program Files (x86)" will exist?
  10. INSTALLER/Windows/X64/Apache.py (Diff revision 2)
     
     
    Show all issues
    Can we assume that this directory exists?
  11. INSTALLER/Windows/X64/CVS.py (Diff revision 2)
     
     
     
    Show all issues
    spaces before *
  12. INSTALLER/Windows/X64/CVS.py (Diff revision 2)
     
     
    Show all issues
    ws
  13. INSTALLER/Windows/X64/CVS.py (Diff revision 2)
     
     
    Show all issues
    Spaces on either side of the +
  14. INSTALLER/Windows/X64/Perforce.py (Diff revision 2)
     
     
     
     
    Show all issues
    spaces on either side of the =
  15. INSTALLER/Windows/X64/PostgreSQL.py (Diff revision 2)
     
     
     
     
    Show all issues
    Spaces on either side of the =
  16. INSTALLER/Windows/X64/Pycrypto.py (Diff revision 2)
     
     
    Show all issues
    Space before the *.
  17. INSTALLER/Windows/X64/Pycrypto.py (Diff revision 2)
     
     
     
     
    Show all issues
    Spaces on either side of the =, +'s
  18. INSTALLER/Windows/X64/Pycrypto.py (Diff revision 2)
     
     
    Show all issues
    Spaces between functions.  Applies to all below.
  19. INSTALLER/Windows/X64/Pycrypto.py (Diff revision 2)
     
     
     
    Show all issues
    Spaces after commas, on either side of =
  20. INSTALLER/Windows/X64/Pycrypto.py (Diff revision 2)
     
     
    Show all issues
    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)
     
     
     
     
     
    Show all issues
    Spaces (see above)
  22. Show all issues
    Spaces for =
  23. Show all issues
    ws
  24. INSTALLER/Windows/X64/SQLite.py (Diff revision 2)
     
     
     
     
    Show all issues
    Still need this?
  25. INSTALLER/Windows/X64/SQLite.py (Diff revision 2)
     
     
     
    Show all issues
    Hm - maybe we'll just use standard escaped quotes instead of those weird characters.
  26. INSTALLER/Windows/X64/download.py (Diff revision 2)
     
     
     
    Show all issues
    I see - we build the folder for them...gotcha.  Do we clean up afterwards?
  27. INSTALLER/Windows/X64/download.py (Diff revision 2)
     
     
    Show all issues
    ws
  28. INSTALLER/Windows/X64/download.py (Diff revision 2)
     
     
    Show all issues
    We should probably be consistent with our capitalization.
  29. INSTALLER/Windows/X64/setpath.py (Diff revision 2)
     
     
    Shorten to <= 80 chars please, and space after commas
  30. INSTALLER/rb-install.py (Diff revision 2)
     
     
     
     
     
     
    Show all issues
    Please put these in alphabetical order
  31. INSTALLER/rb-install.py (Diff revision 2)
     
     
    Show all issues
    Two linebreaks between this from import and the other imports.
  32. INSTALLER/rb-install.py (Diff revision 2)
     
     
     
     
     
    Show all issues
    For these, spaces after the commas.  And the PossibleApplications line needs to be broken up over several lines.
  33. INSTALLER/rb-install.py (Diff revision 2)
     
     
    Show all issues
    Spaces on either side of the = sign
  34. INSTALLER/rb-install.py (Diff revision 2)
     
     
     
     
    Show all issues
    Spaces on either side of the =
  35. INSTALLER/rb-install.py (Diff revision 2)
     
     
    Show all issues
    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.
  36. INSTALLER/rb-install.py (Diff revision 2)
     
     
    Show all issues
    ?
  37. gsoc/Linux/Standards/MySQL.py (Diff revision 2)
     
     
    Show all issues
    ws
  38. gsoc/Linux/Standards/MySQL.py~ (Diff revision 2)
     
     
    Show all issues
    Temp file
  39. gsoc/Linux/Standards/PostgreSQL.py~ (Diff revision 2)
     
     
    Show all issues
    temp file
  40. gsoc/Linux/Standards/PyLucene.py~ (Diff revision 2)
     
     
    Show all issues
    temp file
  41. gsoc/Linux/Standards/SQLite.py~ (Diff revision 2)
     
     
    Show all issues
    temp file
  42. gsoc/rb-install.py (Diff revision 2)
     
     
     
     
     
     
     
     
     
    Show all issues
    Spaces after commas
  43. gsoc/rb-install.py (Diff revision 2)
     
     
     
     
    Show all issues
    Spaces on either side of =
  44. gsoc/rb-install.py (Diff revision 2)
     
     
    Show all issues
    ws
  45. gsoc/rb-install.py (Diff revision 2)
     
     
    Show all issues
    ws
  46. gsoc/rb-install.py (Diff revision 2)
     
     
     
    Show all issues
    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.
  47. gsoc/rb-install.py (Diff revision 2)
     
     
    Show all issues
    I'm seeing lots of "prints" everywhere...shouldn't we be piping to stdout and stderr?
  48. gsoc/rb-install.py (Diff revision 2)
     
     
    Show all issues
    ws
  49. gsoc/rb-install.py~ (Diff revision 2)
     
     
     
    Show all issues
    I think this tilde file is likely temporary, and shouldn't have been included in the review request.
  50. 
      
BB
Review request changed

Status: Discarded

Loading...