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

migrate setup.py to setup.cfg #1553

Merged
merged 7 commits into from
Mar 18, 2024
Merged

Conversation

deronnax
Copy link
Contributor

Setuptools recommends to use declarative setup.cfg over custom imperative setup.py (especially that this one grown some significant body hair)
On the other hand, using data_files (that is used here for man pages) is deprecated and has no direct equivalent in declarative package definition, be it setup.cfg or pyproject.toml, so we might just not be able to replicate the thing.

@deronnax deronnax force-pushed the migrate-to-pyproject-toml branch 3 times, most recently from 4c320b1 to c173a14 Compare January 23, 2024 20:52
@codecov-commenter
Copy link

codecov-commenter commented Jan 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.22%. Comparing base (4d7d6b6) to head (bb13087).
Report is 368 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1553      +/-   ##
==========================================
- Coverage   97.28%   94.22%   -3.07%     
==========================================
  Files          67      113      +46     
  Lines        4235     7705    +3470     
==========================================
+ Hits         4120     7260    +3140     
- Misses        115      445     +330     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jkbrzt
Copy link
Member

jkbrzt commented Mar 4, 2024

@deronnax thanks for the PR! If we find a way to maintain full compatibility including data files, I’ll be happy to merge it.

@deronnax
Copy link
Contributor Author

deronnax commented Mar 5, 2024

Sorry, I was wrong, it's actually doable. There you go. Don't forget to squash merge.
Btw, it probably won't be doable with pyproject.toml, but that's a subject for another day.

@jkbrzt
Copy link
Member

jkbrzt commented Mar 18, 2024

@deronnax thanks!

@jkbrzt jkbrzt merged commit 10b7d31 into httpie:master Mar 18, 2024
38 checks passed
@jkbrzt
Copy link
Member

jkbrzt commented Mar 18, 2024

It looks like the conditional Windows-only stuff like the colorama dependency has not been ported to the new setup.cfg. Tests pass because pytest depends on colorama as well I beliece.

cli/setup.py

Lines 44 to 63 in 3524ccf

install_requires_win_only = [
'colorama>=0.2.4',
]
# Conditional dependencies:
# sdist
if 'bdist_wheel' not in sys.argv:
if 'win32' in str(sys.platform).lower():
# Terminal colors for Windows
install_requires.extend(install_requires_win_only)
# bdist_wheel
extras_require = {
'dev': dev_require,
'test': tests_require,
# https://wheel.readthedocs.io/en/latest/#defining-conditional-dependencies
':sys_platform == "win32"': install_requires_win_only,

jkbrzt added a commit that referenced this pull request Mar 18, 2024
@deronnax
Copy link
Contributor Author

oops sorry. Actually that's tox that's depending colorama.

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.

3 participants