manrajgrover / halo

šŸ’« Beautiful spinners for terminal, IPython and Jupyter
MIT License
2.86k stars 148 forks source link

CHORE: consolidate requirements to setup.py #105

Open theY4Kman opened 5 years ago

theY4Kman commented 5 years ago

Note: this is just a suggestion ā€” I have no horse in this race

Description

This PR moves package requirements out of requirements.txt and straight into install_requires of setup.py (leaving . as the only dep in requirements.txt ā€” which informs pip to look in the setup.py).

Additionally, deps for test_requires were moved out of requirements-dev.txt and into extras_require['test'], leaving halo[test] as the only dep in test_requires.

A new extras_require['dev'] was added, requiring halo[test,ipython], to setup the dev environment. .[dev] is now the only dep in requirements-dev.txt. Requiring the ipython reqs is a new addition to the workflow ā€” it was added, because the linter complains if it can't import ipython stuff.

Why?

All I wanted to do was add -r requirements.txt to requirements-dev.txt, so setting up the dev env would only need pip install -r requirements-dev.txt... then I realized the reqs were read line-by-line in setup.py, so I could either augment the dependencies(req_file_path) method, or ask O Glorious Internet for answers.

Her Majesty, The Internet said _"maybe use extrasrequire['test']". I'd never heard of going that route before (or that pip supports installation of reqs from setup.py using . as a dep), it seemed fun and interesting (for some definitions of "fun" and "interesting"), uhh, so I gave it a shot.

Checklist

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 308


Totals Coverage Status
Change from base Build 304: 0.0%
Covered Lines: 272
Relevant Lines: 289

šŸ’› - Coveralls
manrajgrover commented 5 years ago

@theY4Kman We should also remove requirements file from MANIFEST.in