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

add validator ETH addresses (ERC20) #276

Merged
merged 21 commits into from
Jun 27, 2023
Merged

add validator ETH addresses (ERC20) #276

merged 21 commits into from
Jun 27, 2023

Conversation

msamsami
Copy link
Contributor

This is a validator for ERC20 Ethereum (ETC) addresses.

@yozachar yozachar self-requested a review June 11, 2023 02:15
@yozachar yozachar self-assigned this Jun 11, 2023
@yozachar yozachar added the enhancement Issue/PR: A new feature label Jun 11, 2023
@yozachar yozachar mentioned this pull request Jun 12, 2023
Closed
@msamsami
Copy link
Contributor Author

@joe733
Please merge this PR first and merge #277 after that to avoid conflicts.

yozachar
yozachar requested changes Jun 12, 2023
View reviewed changes
Copy link
Collaborator

@yozachar yozachar left a comment

Choose a reason for hiding this comment

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

@msamsami, please make this PR do one thing. Create another PR to move btc_adderess.py to validators/crypto_addresses.

@msamsami msamsami mentioned this pull request Jun 12, 2023
Closed
@msamsami
Copy link
Contributor Author

@msamsami, please make this PR do one thing. Create another PR to move btc_adderess.py to validators/crypto_addresses.

@joe733
It's done. Here is the correct order for merging the PRs: #276 --> #277 --> #278

yozachar
yozachar requested changes Jun 12, 2023
View reviewed changes
poetry.lock Outdated Show resolved Hide resolved
validators/__init__.py Outdated Show resolved Hide resolved
validators/crypto_addresses/__init__.py Outdated Show resolved Hide resolved
validators/crypto_addresses/eth_address.py Outdated Show resolved Hide resolved
validators/crypto_addresses/eth_address.py Outdated Show resolved Hide resolved
@yozachar yozachar added the waiting Issue/PR: Wating for reply label Jun 13, 2023
@yozachar yozachar removed the waiting Issue/PR: Wating for reply label Jun 17, 2023
yozachar
yozachar requested changes Jun 17, 2023
View reviewed changes
poetry.lock Show resolved Hide resolved
validators/crypto_addresses/eth_address.py Outdated Show resolved Hide resolved
@yozachar yozachar added the waiting Issue/PR: Wating for reply label Jun 17, 2023
@msamsami msamsami requested a review from yozachar June 26, 2023 12:04
@yozachar yozachar removed the waiting Issue/PR: Wating for reply label Jun 27, 2023
yozachar
yozachar requested changes Jun 27, 2023
View reviewed changes
Copy link
Collaborator

@yozachar yozachar left a comment

Choose a reason for hiding this comment

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

Just one more change.

@yozachar yozachar added the waiting Issue/PR: Wating for reply label Jun 27, 2023
@msamsami msamsami requested a review from yozachar June 27, 2023 06:11
@yozachar yozachar removed the waiting Issue/PR: Wating for reply label Jun 27, 2023
yozachar
yozachar requested changes Jun 27, 2023
View reviewed changes
validators/crypto_addresses/eth_address.py Outdated
Comment on lines 4 to 10
# external
import re

from eth_hash.auto import keccak

# local
from validators.utils import validator
Copy link
Collaborator

Choose a reason for hiding this comment

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

@msamsami, I meant the change to be as follows:

# standard
import re

# external
from eth_hash.auto import keccak

# local
from validators.utils import validator

As re is from the standard library, eth_hash.auto.keccak is an external dependency and validators.utils.validator implies a local coupling.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joe733 Sorry for the confusion. It's done. 802487f

@yozachar yozachar added the waiting Issue/PR: Wating for reply label Jun 27, 2023
@yozachar yozachar removed the waiting Issue/PR: Wating for reply label Jun 27, 2023
@yozachar
Copy link
Collaborator

Tests have failed. Ref: https://github.com/python-validators/validators/actions/runs/5387269990/jobs/9778426797?pr=276#step:6:182

Also, please use black to format code. Ref: https://github.com/python-validators/validators/actions/runs/5387269990/jobs/9778426797?pr=276#step:6:203

Instead if you simply run tox, all of this could be resolved, locally.

@yozachar yozachar marked this pull request as draft June 27, 2023 08:12
@msamsami
Copy link
Contributor Author

@joe733 I added backend dependencies of eth-hash and reformatted code using black.

@msamsami msamsami marked this pull request as ready for review June 27, 2023 08:32
@yozachar yozachar merged commit 60c5314 into python-validators:master Jun 27, 2023
6 checks passed
@yozachar
Copy link
Collaborator

@msamsami thanks for the PR!

@yozachar yozachar mentioned this pull request Jul 11, 2023
Closed
@yozachar yozachar mentioned this pull request Aug 7, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yozachar yozachar yozachar approved these changes

Assignees

@yozachar yozachar

Labels
enhancement Issue/PR: A new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@msamsami @yozachar