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

Improvements requirements.txt #133

Closed
wants to merge 2 commits into from

Conversation

mapeveri
Copy link
Contributor

I had some problems getting everything. This is my solution.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.7%) to 84.244% when pulling f584201 on mapeveri:develop into 3e92cd3 on MasoniteFramework:develop.

@coveralls
Copy link

coveralls commented May 15, 2018

Coverage Status

Coverage decreased (-6.0%) to 79.979% when pulling 52ae142 on mapeveri:develop into 3e92cd3 on MasoniteFramework:develop.

@josephmancuso
Copy link
Member

what does this solve? The only thing in requirements.txt should really be only dependencies for development if there are any.

when testing you should do (preferably inside your virtual environment):

$ pip install .

which installs everything what would have installed when downloading the actual package.

or am I missing something?

@mapeveri
Copy link
Contributor Author

I always run pip install -r requirements.txt for develop, In addition there are dependencies that are not in setup.py. We would have to find a way to unify it.

@josephmancuso
Copy link
Member

If you want to unify it you could create a make command for it

@mapeveri
Copy link
Contributor Author

Make command? I thought to do the following:

Create 3 file requirements.

  1. requirements.txt
  2. requirements-test.txt
  3. requirements-dev.txt

And in the setup.py read the content of requirements.txt

What do you think?

@josephmancuso
Copy link
Member

Umm not really a fan. What are some dependencies that are not used in the core package itself. Are there any libraries that are only for development? The solution for this should just be to run pip install . which will install all the required packages

@mapeveri
Copy link
Contributor Author

Yes, for example:

For tests:

pytest==3.3.1
coveralls

For dev:

boto3==1.5.24
pusher==1.7.4
ably==1.0.1

And in the setup.py the dependencie cryptography is duplicated.

pip install . only install dependencies of masonite, but not dev and tests dependencies.

@josephmancuso
Copy link
Member

Oh right yeah the only thing that should be the requirements.txt file is those dependencies. If we put the dependencies in both the setup.py and requirements.txt then we need to make sure both are always up to date.

@mapeveri
Copy link
Contributor Author

then how do we continue?

@josephmancuso
Copy link
Member

Just do:

pip install .
pip install -r requirements.txt

Requirements.txt just needs the ably, pusher, boto3 and coveralls packages

@mapeveri
Copy link
Contributor Author

And now @josephmancuso what do you think?

@josephmancuso
Copy link
Member

josephmancuso commented May 16, 2018

how does the install.sh work?

./install.sh?

@mapeveri
Copy link
Contributor Author

Simplely run this together:

pip install .
pip install -r requirements.txt

For executed run:

sh install.sh

@josephmancuso
Copy link
Member

josephmancuso commented May 17, 2018

hmm I'm sorry. After some thinking I just feel like adding a file to run a test just to combine two commands seems silly to me. This will only be useful when someone adds a package to the repo which really won't be that often.

I'll open if circumstances change

@mapeveri
Copy link
Contributor Author

It is not only to unify two commands. It is also for:

  1. Remove dependencies from requirements.txt and leave only those in setup.py
  2. Remove duplicate dependencies in setup.py
  3. In requirements.txt leave only the 'dev' and 'tests' dependency.

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

3 participants