halo icon indicating copy to clipboard operation
halo copied to clipboard

CHORE: consolidate requirements to setup.py

Open theY4Kman opened this issue 6 years ago • 2 comments

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 extras_require['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

  • [X] Your branch is up-to-date with the base branch
  • [X] ~You've included at least one test if this is a new feature~
  • [X] All tests are passing

theY4Kman avatar Oct 27 '18 20:10 theY4Kman

Pull Request Test Coverage Report for Build 308

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 92.157%

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

💛 - Coveralls

coveralls avatar Oct 27 '18 20:10 coveralls

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

manrajgrover avatar Oct 30 '18 18:10 manrajgrover