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

🔨 Re-organize dependencies for local dev and CI #706

Merged
merged 1 commit into from
Feb 3, 2024

Conversation

sergei-maertens
Copy link
Contributor

The 'hard' dependencies are now specified as extra groups in the setup.py package information. tox.ini points to these extras to install the required dependencies, and their supported version ranges.

The dev requirements file is now documented in the contributing guidelines, and makes use of the extras to pull in the optional dependencies for a full-blown local installation. The remaining dependencies (such as sphinx) are not used in tox and are still in the requirements_dev.txt file.

The linting dependencies (with upper bound versions) are now also specified as an extra group to prevent mismatching versions in tox env and local development envs.

Motivation and Context

pip install -r requirements_dev.txt lead to different versions of isort/flak8 being installed in local env and CI pipeline. While it
didn't cause problems yet, these are annoying to track down.

Additionally, when touching minimum required versions, it's hard to keep track of where the version numbers are all specified
and can lead to false negatives in the tests if there are things broken because one specification overrides another.

How Has This Been Tested?

  • Clean virtualenv
  • pip install -r requirements_dev.txt
  • make example-webauthn
  • tox

Screenshots (if appropriate):

n/a

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Project maintenance

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

The 'hard' dependencies are now specified as extra groups in the
setup.py package information. tox.ini points to these extras to
install the required dependencies, and their supported version ranges.

The dev requirements file is now documented in the contributing
guidelines, and makes use of the extras to pull in the optional
dependencies for a full-blown local installation. The remaining
dependencies (such as sphinx) are not used in tox and are still
in the requirements_dev.txt file.

The linting dependencies (with upper bound versions) are now
also specified as an extra group to prevent mismatching
versions in tox env and local development envs.
Copy link

codecov bot commented Feb 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6f3bbd8) 97.83% compared to head (fe9dab5) 97.83%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #706   +/-   ##
=======================================
  Coverage   97.83%   97.83%           
=======================================
  Files          78       78           
  Lines        3378     3378           
  Branches      376      376           
=======================================
  Hits         3305     3305           
  Misses         42       42           
  Partials       31       31           

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

Copy link
Contributor

@claudep claudep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks a lot! Sooner or later, we should also migrate to pyproject.toml instead of setup.py.

@claudep claudep merged commit 13a0f80 into master Feb 3, 2024
@claudep claudep deleted the chore/consolidate-dependencies-management branch February 3, 2024 17:22
@sergei-maertens
Copy link
Contributor Author

yeah, I wanted to mention that earlier but it involves some changes in build tooling too. we recently updated our own starter template to it, maybe @Viicos wants to do this one 😉

@Viicos
Copy link
Contributor

Viicos commented Feb 3, 2024

Sure thing, always happy to migrate projects to the new PEP 517/8

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