Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Pipfile to the project #933

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add Pipfile to the project #933

wants to merge 2 commits into from

Conversation

blackHatMonkey
Copy link
Member

@blackHatMonkey blackHatMonkey commented Jun 1, 2018

Add Pipfile and Pipfile.lock to the project. This should make dependency management much easier. Also testing process with tox and travis have been changed to take advantage of Pipfile. Also a Makefile is added to make it easier to run commands. For example for running test all you need to do is run

make test

and this could be integrated into text-editors and IDEs.

Add Pipfile and Pipfile.lock to the project. This should make dependancy
managment much easier. Also testing process with tox and travis have
been changed to take advantage of Pipfile.
@blackHatMonkey
Copy link
Member Author

@sophron Can we proceed with this?

@sophron
Copy link
Member

sophron commented Jun 26, 2018

Hi @blackHatMonkey,

I don't see any Makefile added.

Also, a couple of questions:

  • What is the exact purpose of Pipfile and Pipfile.lock? I did read this, but I'm wondering the benefits of adding them in our project.

  • What is the maintenance cost from our side? When and how should we update these files?

Thanks!

@blackHatMonkey
Copy link
Member Author

I don't see any Makefile added.

Since we are using tox I decided it was overkill but you can see it in here cda686e .

What is the exact purpose of Pipfile and Pipfile.lock? I did read this, but I'm wondering the benefits of adding them in our project.

I suggest watching this as he explains it better than I ever could but in essence it allows for a completely reproducible environments and builds. So it should make our testing and in the future deployment easier.

What is the maintenance cost from our side? When and how should we update these files?

As far as maintenance goes we can update them as often as we need to but maybe once a month should be more than enough. All we have to do is run pipenv update followed by testing tox and if successful push the new lock file.

@sophron
Copy link
Member

sophron commented Jun 28, 2018

Introducing new functionalities and tools is not as simple as it sounds. It may indeed solve a few problems but it will also raise the complexity of managing the project. And remember, that we are volunteers that have not signed any kind of contract.

Having said that, I'm not against this PR but I will have to understand the reasoning behind this.

So it should make our testing and in the future deployment easier.

In what way it will make it easier? What are the problems with our current testing and our current deployment that it will solve? And what do you mean by "future deployment"?

@blackHatMonkey
Copy link
Member Author

pipenv will eventually be integrated in to pip and it is the recommended way of managing dependencies from python.org.

The current testing setup is pretty good but it has one downside which is you can't do functional testing with it. This means that to fully test any changes you either must have a kali machine or a virtual machine. With pipenv I could just run pipenv shell and it would give a shell into the virtual environment. I would do the testing and I don't have to install anything on my system except non python dependencies. Then I can remove the environment and the next time pipenv will give me the exact same environment for testing.

@sophron
Copy link
Member

sophron commented Jun 29, 2018

Okay, I now see that pipenv is pip and virtualenv combined. It looks like it can make our whole setup process way smoother. The fact that is backed by python and is going to replace pip is also important.

What I expect however is to be able to do pipenv install -e git+https://github.com/wifiphisher/wifiphisher.git and have everything ready. This is not currently the case because our setup.py messes with stdin and this is something that we should fix.

Brian, do you think you can make these fixes and update the PR?

PS: Kudos to @alexkiousis that quickly demonstrated virtualenv to me.

@blackHatMonkey
Copy link
Member Author

@sophron I think it would be better to have a separate PR for that and after that we can merge this. What do you think?

blackHatMonkey added a commit that referenced this pull request Jun 30, 2018
Move the dependency checks from the setup.py file to it's own module so building
the project becomes easier. This is due to the fact of waiting on stdin in case
exteranl dependency was not installed. The checks now happen at run time.

seealso #933
@sophron
Copy link
Member

sophron commented Jul 2, 2018

Let's work on merging #960 first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants