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

Update black command to use regex pattern #2727

Merged
merged 1 commit into from
Nov 22, 2022

Conversation

fselmo
Copy link
Collaborator

@fselmo fselmo commented Nov 22, 2022

What was wrong?

  • Running black locally via tox or make lint has never really worked for me. This regex way to set up the black command via tox seems to work a bit better locally and regex seems to be the recommended way to run the command.

Todo:

Cute Animal Picture

20221116_090946

@fselmo fselmo marked this pull request as ready for review November 22, 2022 00:42
@pacrob
Copy link
Contributor

pacrob commented Nov 22, 2022

regex seems to be the recommended way to run the command.

Could you add a link to your reference? Thanks!

@fselmo
Copy link
Collaborator Author

fselmo commented Nov 22, 2022

Could you add a link to your reference? Thanks!

Clicking the CLI reference down arrow here:

Options:
    ...
    
    --exclude TEXT                A regular expression that matches files and
                                  directories that should be excluded on
                                  recursive searches. An empty value means no
                                  paths are excluded. Use forward slashes for
                                  directories on all platforms (Windows, too).
                                  Exclusions are calculated first, inclusions
                                  later. [default: /(\.direnv|\.eggs|\.git|\.h
                                  g|\.mypy_cache|\.nox|\.tox|\.venv|venv|\.svn
                                  |\.ipynb_checkpoints|_build|buck-
                                  out|build|dist|__pypackages__)/]
                                  
    --extend-exclude TEXT         Like --exclude, but adds additional files
                                  and directories on top of the excluded ones.
                                  (Useful if you simply want to add to the
                                  default)

@fselmo
Copy link
Collaborator Author

fselmo commented Nov 22, 2022

@pacrob come to think of it, we could probably just use --exclude like so:

black {toxinidir}/ens {toxinidir}/ethpm {toxinidir}/web3 {toxinidir}/tests {toxinidir}/setup.py --exclude /ethpm/ethpm-spec/|/ethpm/_utils/protobuf/ipfs_file_pb2\.py --check

Any preference between the two?

@pacrob
Copy link
Contributor

pacrob commented Nov 22, 2022

Any preference between the two?

Slight preference for the latter

@fselmo
Copy link
Collaborator Author

fselmo commented Nov 22, 2022

Slight preference for the latter

Same. I'll update the commit.

- Use regex pattern for ``black`` cli command via ``tox``. This fixes issues that I was experiencing locally and seems to be the recommended way of using the ``--exclude`` flag.
Copy link
Contributor

@pacrob pacrob left a comment

Choose a reason for hiding this comment

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

lgtm!

@fselmo fselmo merged commit c7d2c2c into ethereum:master Nov 22, 2022
fselmo added a commit to fselmo/web3.py that referenced this pull request Nov 22, 2022
- Due to similar issues as was happening in ethereum#2727, ``mypy`` looks for regular expressions which led to issues running ``make lint`` locally.
fselmo added a commit to fselmo/web3.py that referenced this pull request Nov 22, 2022
- Due to similar issues as was happening in ethereum#2727, ``mypy`` looks for regular expressions which led to issues running ``make lint`` locally.
fselmo added a commit that referenced this pull request Nov 23, 2022
- Due to similar issues as was happening in #2727, ``mypy`` looks for regular expressions which led to issues running ``make lint`` locally.
@fselmo fselmo deleted the update-black-command branch April 3, 2024 20:51
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.

2 participants