-
Notifications
You must be signed in to change notification settings - Fork 375
chore(deps,tooling): move dev extras to dependency-group
#1688
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
chore(deps,tooling): move dev extras to dependency-group
#1688
Conversation
- `min_version` is deprecated: https://tox.wiki/en/latest/config.html#min_version
6539bcd to
bd5dbd3
Compare
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/osaka #1688 +/- ##
============================================
Coverage 86.07% 86.07%
============================================
Files 743 743
Lines 44078 44078
Branches 3894 3894
============================================
Hits 37938 37938
Misses 5659 5659
Partials 481 481
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Simplify tox.ini by always installing all dev dependencies.
danceratopz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed up a packaging bug in testing that cropped up due to these changes; it was missing ruff as a dependency.
This reverts commit 661a1b5. This will be handled differently: ethereum#1715
91d3c92 to
073779f
Compare
gentest still requires ruff, cf ethereum#1715
fselmo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm once CI passes 👍🏼
Thanks, now we're failing testing unit tests (here) due to the issue that @felix314159 is addressing here |
|
Yep sounds good on CI fix in separate PR if you're good with it 👍🏼 |
🗒️ Description
tldr
testingpackage by addingruffas a dep (required bygentest).What changes?
Developers need to run:
to get all dev dependencies (previously it was
uv sync --all-extras).More Background
These changes follow a retroactive review of #1671, sorry, I was too slow @fselmo!
I think the standard way of adding
devdependencies is not as an "extra", but rather as adependency-group:So this PR makes that change and additionally removes
toxas a directdevdependency; imo, it should be ran as a "tool" viauvx; the doc changes already made inreflect that.
Before this PR:
With this PR:
It's great that we get to drop the
--with=tox-uvflag!🔗 Related Issues or PRs
N/A.
✅ Checklist
toxchecks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx tox -e statictype(scope):.All: Considered adding an entry to CHANGELOG.md.skippedCute Animal Picture