14
\$\begingroup\$

I want to backup my GitHub repositories since there is news about blocking GitHub in India. I wrote Python code to automate this and would like for it to be reviewed.

import os
import requests

API_TOKEN='xxxxx'
#from https://github.com/settings/applications

api_repo='https://api.github.com/user/repos'

url = '%s?access_token=%s' % \
(api_repo,API_TOKEN,)

r = requests.get(url).json()
git_urls = [repo['git_url'] for repo in r]
for i in git_urls:
  os.system("git clone "+i) 
\$\endgroup\$
14
\$\begingroup\$

requests is third-party, so (per the style guide) there should be a blank line in the imports:

import os

import requests

api_repo is constant, so should be API_REPO. There should also be spaces around the = in assignments:

API_REPO = 'https://api.github.com/user/repos'

Explicit line continuation isn't very Pythonic, especially when the line is short enough anyway:

url = '%s?access_token=%s' % \
(api_repo,API_TOKEN,)

Also, there should be spaces after commas (and I wouldn't bother with the trailing one). This would be better written as:

url = '%s?access_token=%s' % (API_REPO, API_TOKEN)

r and i aren't very good variable names. I would use req_json and git_url.


You are mixing % string formatting with + concatenation. You should at least be consistent, and I would use the more modern str.format:

os.system("git clone {}".format(git_url)) 

You should also be consistent with string literal quotes. Per the style guide:

In Python, single-quoted strings and double-quoted strings are the same. This PEP does not make a recommendation for this. Pick a rule and stick to it. When a string contains single or double quote characters, however, use the other one to avoid backslashes in the string. It improves readability.


In all, I would probably have written it as:

import os

import requests

API_TOKEN = "..."
API_URL = "https://api.github.com/user/repos"

url = "{}?access_token={}".format(API_URL, API_TOKEN)
req_json = requests.get(url).json()

for repo in req_json:
    os.system("git clone {}".format(repo["git_url"]))
\$\endgroup\$
  • 1
    \$\begingroup\$ There is a stray access_token parameter in API_URL which should be removed I think. \$\endgroup\$ – ChrisWue Jan 1 '15 at 19:05
  • \$\begingroup\$ Where in the style guide does it talk about blank lines between imports? I just read over it but I seem to have missed that. \$\endgroup\$ – David Z Jan 2 '15 at 4:10
  • 1
    \$\begingroup\$ pylint pep8 doesn't find it, but I've been doing something similar; std imports [\n\n] lib imports [\n\n] proj imports \$\endgroup\$ – Wyrmwood Jan 2 '15 at 4:16
  • 2
    \$\begingroup\$ @DavidZ: python.org/dev/peps/pep-0008/#imports Section "Imports", second bullet, second paragraph. \$\endgroup\$ – unperson325680 Jan 2 '15 at 6:47
  • 2
    \$\begingroup\$ You forgot mixing quote styles. Even your example has a single-quoted string in one place. \$\endgroup\$ – nyuszika7h Jan 2 '15 at 10:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.