Closed ivanlei closed 9 years ago
Wow! :heart:
Need to send CLA.
I will be reviewing this in the next day or so, but let me know about the CLA @alexcpsec.
And thanks @ivanlei !! :bow:
CLA is signed
On Sun, May 3, 2015 at 5:29 PM, Kyle Maxwell notifications@github.com wrote:
I will be reviewing this in the next day or so, but let me know about the CLA @alexcpsec.
And thanks @ivanlei !! :bow:
Reply to this email directly or view it on GitHub:
https://github.com/mlsecproject/combine/pull/138#issuecomment-98560056
This e-mail message and any files transmitted with it contain legally privileged, proprietary information, and/or confidential information, therefore, the recipient is hereby notified that any unauthorized dissemination, distribution or copying is strictly prohibited. If you have received this e-mail message inappropriately or accidentally, please notify the sender and delete it from your computer immediately.
Thanks, this was fantastic. I'd started to work locally on a package, so you saved me a lot of work (not to mention all the other useful stuff here!)
This change effectively builds combine as a proper python package.
setup.py
This add a distutils compatible
setup.py
. combine is now a real and easy to install python package.combine/
dir.setup.py
and incombine/combine.py
. Having it in just 1 place would be ideal.combine
package is installed, there is no need to$ python combine.py
anymore. From any directory just use$ combine
(or$ reaper
,$ thresher
,$ winnower
,$baler
). Magic makes sure the right python code is called.tox
tox is a super popular way to build virtualenvs and run tests. This change adds a tox.ini that both builds virtualenvs and runs tests.
Makefile
that mostly just calls tox$ make venv
- creates a virtualenv invenv-combine
. Enter the virtualenv with$ source venv-combine/bin/activate
$ make test
- creates a virtualenv in.tox
, runs all the tests, runs code coverage, and exits.grequests
to properly install whether manually running$ pip install -r requirements.txt
or$make test
took some doing.--allow-external grequests
topip install
was necessary.-e .
param inrequirements-dev.txt
seems to not inherit--allow-external
so I used-r requirements.txt
instead which magically worked. I only wasted like 90 min figuring that out.pre-commit
pre-commit is a simple tool for ensuring code quality and finding bugs. It adds git hooks that validate the code. After building a virtualenv run
$ pre-commit install
docker
These changes have been tested in docker. The
Dockerfile
and docker readme have been updated. The existing docker stuff was broken prior to these changes.functional code changes
There are very few functional code changes
combine.py
--version
has been added.--output-dir
has been added.