8
\$\begingroup\$

I implemented a Python script that generates git commits for the last year making one's Contribution Graph look full of changes. It was my first Python program (I work mostly with Java). Although the program is quite useless I want to use it as a learning playground to build my Python competences. That is why I would appreciate any feedback about the code (both constructive and destructive).

Questions:

  1. I didn't use types to make the script work with Python 2 and 3. Is it a good practice?
  2. Does the script follow Python style conventions? I didn't manage to find unified Python style guides, I used what was provided by flake8.
  3. Do you usually cover scripts like this with Unit tests? How would you do that?
  4. And generally what would you improve? Any comments are appreciated.

Code: (applied the changes proposed by Graipher)

You can find the full script here .

def main():
 args = arguments()
 curr_date = datetime.now()
 directory = 'repository-' + curr_date.strftime('%Y-%m-%d-%H-%M-%S')
 repository = args.repository
 if repository is not None:
 start = repository.rfind('/') + 1
 end = repository.rfind('.')
 directory = repository[start:end]
 no_weekends = args.no_weekends
 frequency = args.frequency
 os.mkdir(directory)
 os.chdir(directory)
 run(['git', 'init'])
 start_date = curr_date.replace(hour=20, minute=0) - timedelta(366)
 for day in (start_date + timedelta(n) for n in range(366)):
 if (not no_weekends or day.weekday() < 5) \
 and randint(0, 100) < frequency:
 for commit_time in (day + timedelta(minutes=m)
 for m in range(contributions_per_day(args))):
 contribute(commit_time)
 if repository is not None:
 run(['git', 'remote', 'add', 'origin', repository])
 run(['git', 'push', '-u', 'origin', 'master'])
 print('\nRepository generation ' +
 '\x1b[6;30;42mcompleted successfully\x1b[0m!')
def contribute(date):
 with open(os.path.join(os.getcwd(), 'README.md'), 'a') as file:
 file.write(message(date) + '\n\n')
 run(['git', 'add', '.'])
 run(['git', 'commit', '-m', '"%s"' % message(date),
 '--date', date.strftime('"%Y-%m-%d %H:%M:%S"')])
def run(commands):
 Popen(commands).wait()
def message(date):
 return date.strftime('Contribution: %Y-%m-%d %H:%M')
def contributions_per_day(args):
 max_c = args.max_commits
 if max_c > 20:
 max_c = 20
 if max_c < 1:
 max_c = 1
 return randint(1, max_c)
def arguments():
 parser = argparse.ArgumentParser()
 parser.add_argument('-nw', '--no_weekends',
 required=False, action='store_true', default=False,
 help="""do not commit on weekends""")
 parser.add_argument('-mc', '--max_commits', type=int, default=10,
 required=False, help="""Defines the maximum amount of
 commits a day the script can make. Accepts a number
 from 1 to 20. If N is specified the script commits
 from 1 to N times a day. The exact number of commits
 is defined randomly for each day. The default value
 is 10.""")
 parser.add_argument('-fr', '--frequency', type=int, default=80,
 required=False, help="""Percentage of days when the
 script performs commits. If N is specified, the script
 will commit N%% of days in a year. The default value
 is 80.""")
 parser.add_argument('-r', '--repository', type=str, required=False,
 help="""A link on an empty non-initialized remote git
 repository. If specified, the script pushes the changes
 to the repository. The link is accepted in SSH or HTTPS
 format. For example: [email protected]:user/repo.git or
 https://github.com/user/repo.git""")
 return parser.parse_args()
if __name__ == "__main__":
 main()
Ben A
10.7k5 gold badges37 silver badges101 bronze badges
asked Aug 29, 2019 at 15:03
\$\endgroup\$
4
  • 4
    \$\begingroup\$ Welcome to Code Review! This site has a very generous limit on the amount of code you can put in a question. Since you are ought to have all the code that you want to have reviewed in the question and your script is not super massive, it's likely best to show it in full length. \$\endgroup\$ Commented Aug 29, 2019 at 15:35
  • 2
    \$\begingroup\$ @AlexV added the full script \$\endgroup\$ Commented Aug 30, 2019 at 20:36
  • \$\begingroup\$ You say that you support Python 2 because it's your OS default, but... that's not great. If you write a shebang with python3, your script will use the right thing and your code can be improved. \$\endgroup\$ Commented Sep 2, 2019 at 14:43
  • \$\begingroup\$ To anyone looking at the edit history: I rolled it back after seeing that the OP changed the code, then realizing that they added the full script at the request of @AlexV, I rolled the post back to the updated code. \$\endgroup\$ Commented Sep 4, 2019 at 4:25

1 Answer 1

7
+50
\$\begingroup\$

Python is actually well-known for having a unified style-guide. It is called PEP8. It has both general advice as well as specific advice (naming and such). flake8 follows the recommendations of PEP8, AFAIK.

Currently your script only runs on UNIX system due to the way you manually handle file paths for the directory variable. If you were consistently using the os.path module instead, it would run also on Windows.

import os
if args.repository is not None:
 directory = os.path.splitext(os.path.basename(args.repository))[0]
else:
 directory = 'repository-' + curr_date.strftime('%Y-%m-%d-%H-%M-%S')

I would add a os.chdir(directory) somewhere in the beginning. This way you don't need to pass the directory to every call of perform. I would leave it as an argument (in case you do need it), but make it optional. I would also call the function run.

def run(commands, context_dir=None):
 if context_dir is None:
 context_dir = os.getcwd()
 Popen(commands, cwd=context_dir).wait()

Actually, that is already the default behaviour of Popen!. So you can just do:

def run(commands, context_dir=None):
 Popen(commands, cwd=context_dir).wait()

In your argument parsing a few things are superfluous. While "explicit is better than implicit", "simple is better than complex". All arguments that start with a - or -- are automatically optional, so you don't need the required=False. In addition, with action="store_true", default=False is implied.

type=str is also redundant, since that is the default for all arguments.

The repository option should maybe have a nargs=1 keyword to ensure an actual argument is passed.

I would pass an optional argument to the parsing, which let's you test the function with a list of strings:

def arguments(args=None):
 ....
 return parser.parse_args(args)
print(arguments(["-r"]))

If you were not concerned about Python 2 compatibility (it's time is finally coming to an end, slowly), I would have recommended using the new high-level API subprocess.run (Python 3.5+, 3.7+ for some features). With that your function would not even be needed anymore, a simple from subprocess import run would be enough.

In that vain, you could then also use pathlib, as recommended in the comments by @grooveplex:

from pathlib import Path
if args.repository is not None:
 directory = Path(args.repository).stem
...
answered Aug 29, 2019 at 17:10
\$\endgroup\$
3
  • 3
    \$\begingroup\$ And if you're not concerned about Python 2 compatibility, you could also use pathlib, which is really quite nice. \$\endgroup\$ Commented Aug 29, 2019 at 17:23
  • 2
    \$\begingroup\$ @grooveplex Added a short part on that. \$\endgroup\$ Commented Aug 29, 2019 at 17:29
  • 1
    \$\begingroup\$ @Graipher I applied most of your recommendations and added the full script to the question. I kept Python 2 compatibility simply because my latest Ubuntu LTS still defaults to python 2. As for Windows compatibility, I was not clear enough - the args.repository is a git repository link (either HTTPS or SSH format). It should also work on Windows (though, I had no chance to test it). \$\endgroup\$ Commented Aug 30, 2019 at 20:45

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.