search

gophish / api-client-python

A Python API Client for Gophish
MIT License
44 stars 48 forks source link

Include required dependencies in setup.py #16

Closed felddy closed 5 years ago

felddy commented 5 years ago

This pull request corrects issue #15

With the dependencies enumerated in the install_requires section of setup.py users will now install the all the required packages when using pip install gophish.

Developers of this project can now initialize their working environment using pip install -r requirements-dev.txt from the root of the project.

For more information on the differences between the two locations please see the Python Packaging User Guide: install_requires vs requirements files.

setup.py was also blacked during the modification.

bjb28 commented 5 years ago

Nice fix @felddy

dav3r commented 5 years ago

Ping, is anybody out there? @jordan-wright - are you able to review this PR?

jordan-wright commented 5 years ago

Sorry for the delay everyone.

I like the change of moving things out of requirements.txt and into setup.py. Thanks for that @felddy!

However, I'm not sure I agree with the changes to requirements.txt, and requirements-dev.txt. I don't personally use ipython when developing this library, so I'd like to avoid making this a requirement.

I'm of the opinion that we move everything to setup.py as you did, and just remove the existing requirements.txt files until we have a need otherwise (e.g. to support a particular linter such as black, which I'd be ok with - I've personally been using yapf).

jordan-wright commented 5 years ago

More than anything, I do want to express my gratitude for you taking the time to make these changes. Great work!

quelsan commented 5 years ago

Is there anything stopping this PR from being merged, @jordan-wright ? If you are not sure about the dev requirements just yet (ipython), I suggest that you do not include that specific commit in merge.

Would be happy to see a new release tagged and pushed :-)

felddy commented 5 years ago

@jordan-wright, just tickling this PR.

I wasn't sure if you saw the last commit seven days ago: https://github.com/gophish/api-client-python/pull/16/commits/0eff1695b6a6944c8c18ef13bd745fe095a921db

jordan-wright commented 5 years ago

My apologies for the delay @felddy! I've been swamped with all sorts of things. This LGTM. Thanks for taking the time to send this in!

I'll see if I can get a new release tagged soon.