search

cuspaceflight / firefish

CFD simulation software for Martlet 3
Apache License 2.0
5 stars 4 forks source link

Adding Kinematics Python Program #37

Closed lwcook closed 8 years ago

lwcook commented 8 years ago

Kinematics program written in python (by lwc24).

jb803 commented 8 years ago

If you do 'git pull upstream master' and then do another commit and push then that might trigger it to do the checks?

rjw57 commented 8 years ago

Yeah, looks like this branch drifted out of sync with master.

jb803 commented 8 years ago

:+1:

rjw57 commented 8 years ago

So this looks good to me with the following comments.

  1. Why is "Kinematics.py" in the root of the repo? I'd have expected it to be under examples like similar scripts.
  2. Python scripts conventionally don't use capitalisation in filenames.
  3. There should be a docstring at the beginning of the program describing it. (See other examples.)
  4. Ideally the docstring should include a doctest to run the program.

I'd ideally not have merged this. I'll open a PR which addresses the more serious issues.

rjw57 commented 8 years ago

So, it would've been a :-1: from me as is.

rjw57 commented 8 years ago

Good work on the script, though. It's just not where I'd have put it :).