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.

Sign up for GitHub

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

Jump to bottom

remove unused docs deps, bump remaining #2890

Merged
merged 4 commits into from
Mar 23, 2023
Merged

remove unused docs deps, bump remaining #2890

merged 4 commits into from
Mar 23, 2023

Conversation

pacrob
Copy link
Contributor

@pacrob pacrob commented Mar 22, 2023 •
edited
Loading

What was wrong?

Some docs dependencies aren't used, others can be bumped.

How was it fixed?

Removed and bumped versions.
While here, removed deprecated use of distutils Version class.

Todo:

Cute Animal Picture

image

@pacrob pacrob force-pushed the cleanup-docs-dependencies branch 3 times, most recently from 7bb977a to df0ddd2 Compare March 22, 2023 21:17
@pacrob pacrob force-pushed the cleanup-docs-dependencies branch from df0ddd2 to f6726ca Compare March 22, 2023 21:20
@pacrob pacrob requested review from kclowes and fselmo March 22, 2023 21:33
fselmo
fselmo previously approved these changes Mar 22, 2023
View reviewed changes
Copy link
Collaborator

@fselmo fselmo left a comment

Choose a reason for hiding this comment

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

I absolutely love this. Thank you 🥇 We've neglected this cleanup for too long.

@kclowes
Copy link
Collaborator

kclowes commented Mar 22, 2023 •
edited
Loading

If you're feeling up for it, there is a deprecation warning when running make docs in web3/_utils/normalizers.py that I believe can just be removed. Also fine kicking it down the road too. Warning looks like: web3.py/web3/_utils/normalizers.py:242: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead. if LooseVersion(eth_abi.__version__) < LooseVersion("2"):

@pacrob pacrob force-pushed the cleanup-docs-dependencies branch from 730e8c7 to c91b511 Compare March 23, 2023 16:53
@fselmo fselmo dismissed their stale review March 23, 2023 16:58

more changes are coming in, will review again

fselmo
fselmo reviewed Mar 23, 2023
View reviewed changes
Makefile Outdated Show resolved Hide resolved
@pacrob pacrob force-pushed the cleanup-docs-dependencies branch from c91b511 to f6726ca Compare March 23, 2023 17:05
@pacrob
Copy link
Contributor Author

pacrob commented Mar 23, 2023

If you're feeling up for it, there is a deprecation warning when running make docs in web3/_utils/normalizers.py that I believe can just be removed. Also fine kicking it down the road too. Warning looks like: web3.py/web3/_utils/normalizers.py:242: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead. if LooseVersion(eth_abi.__version__) < LooseVersion("2"):

I'm not able to reproduce locally. Is it somewhere in CI?

@kclowes
Copy link
Collaborator

kclowes commented Mar 23, 2023

Ah, I was checking DeprecationWarnings and have my PYTHONWARNINGS environment variable set to always or something. You should be able to reproduce if you do an export PYTHONWARNINGS=always. Here's some more info if you're curious.

@pacrob pacrob force-pushed the cleanup-docs-dependencies branch from ed58196 to ddb1aa4 Compare March 23, 2023 20:55
@pacrob pacrob force-pushed the cleanup-docs-dependencies branch from ddb1aa4 to e1dbffa Compare March 23, 2023 20:57
@pacrob pacrob requested a review from fselmo March 23, 2023 21:21
kclowes
kclowes approved these changes Mar 23, 2023
View reviewed changes
Copy link
Collaborator

@kclowes kclowes left a comment

Choose a reason for hiding this comment

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

🎉

fselmo
fselmo approved these changes Mar 23, 2023
View reviewed changes
Copy link
Collaborator

@fselmo fselmo left a comment

Choose a reason for hiding this comment

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

lgtm 👍🏼

@pacrob pacrob merged commit 8f51801 into ethereum:master Mar 23, 2023
@pacrob pacrob deleted the cleanup-docs-dependencies branch March 23, 2023 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kclowes kclowes kclowes approved these changes

@fselmo fselmo fselmo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pacrob @kclowes @fselmo