[llvm-commits] [Psuedo-PATCH] Getting Started

Eli Bendersky eliben at google.com
Thu Nov 29 15:32:46 PST 2012


> What's a good next step?
>
> Joe
>

Some comments on the code:

+
+verbose = 0
+llvm_url = "http://llvm.org/svn/llvm-project/"
+llvm_branch = "trunk"
+llvm_revision = ""
+svn_command = "svn"
+svn_quiet = "-q"
+

If these are constants, then put them in uppercase (in general, follow
PEP 8 for Python code) and you don't really need to (read: please
don't) use "global" when accessing them. Refrain from global variables
you need to modify from within functions.

+def usage () :
+    print ""
+    print "LLVM Getting Started (See http://llvm.org/docs/GettingStarted.html)"
+    print "./getttingStarted.sh"
+    print ""
+    print "By default

It would be easier to just have a single triple-quoted string printed,
instead of calling print many times.
Also using either optparse or argparse could help you generate this
usage string automatically from the options, without the need for
duplication.

+    url = llvm_url + project + '/' + llvm_branch + llvm_revision

Prefer urlparse.urljoin

+    if verbose == 1:
+        os.spawnlp(os.P_WAIT, svn_command, svn_command, "checkout",
url, target)
+    else:
+        os.spawnlp(os.P_WAIT, svn_command, svn_command,
+           svn_quiet, "checkout", url, target)

Please use subprocess instead of os.spawn*. Read
http://docs.python.org/2.7/library/subprocess.html#replacing-older-functions-with-the-subprocess-module
Also, in terms of logic you don't really need to duplicate the
os.spawnlp call here, right?

+    print "./getttingStarted.sh"

Update to .py


Eli



More information about the llvm-commits mailing list