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

Remove black option --diff #6594

Merged

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented May 27, 2022

One of the last remains tasks of #4776
Closes #5079

Remove --diff option which currently prevents black from re-formatting modified Python files.

I only made changes to .pre-commit-config.yaml. Pre-commit CI automatically made the changes to all other files.

After Slacking with @mekarpeles, we will move the following question to a separate PR.

Question: If we are going to modify 40 Python files, given #2981 (comment) should we modify even more by:

  1. Dropping --skip-string-normalization and allowing black to favor double-quoted strings in Python code. -- OR --
  2. Adding double-quote-string-fixer to prefer single-quotes in Python files instead of black-standard double-quotes. See: pre-commit: Should we favor single quoted strings in Python? #6602

Technical

Once this is landed we should establish a flake8 baseline against:
flake8 **/*.py --exclude="./.*,./vendor/*" --ignore=E203,W503 --max-complexity=41 --max-line-length=1195

Testing

Screenshot

Stakeholders

@cclauss cclauss changed the title Remove black option diff again Remove black option --diff again May 27, 2022
@cclauss cclauss force-pushed the remove_black_option_--diff_again branch from eb549c8 to 35400a3 Compare May 27, 2022 12:15
@cclauss cclauss requested review from mekarpeles, cdrini and jimchamp May 27, 2022 12:19
@cclauss cclauss force-pushed the remove_black_option_--diff_again branch from aed7811 to 410f6a2 Compare May 27, 2022 12:44
@cclauss cclauss changed the title Remove black option --diff again Remove black option --diff May 27, 2022
@cclauss cclauss force-pushed the remove_black_option_--diff_again branch from e99a734 to 410f6a2 Compare May 28, 2022 10:14
@cclauss cclauss force-pushed the remove_black_option_--diff_again branch from 7a4a206 to df4fd16 Compare May 31, 2022 06:02
@cclauss cclauss force-pushed the remove_black_option_--diff_again branch from 39fa537 to e16adf4 Compare June 1, 2022 16:43
@cclauss cclauss force-pushed the remove_black_option_--diff_again branch from 48fa6b8 to ce03f52 Compare June 9, 2022 06:32
@cclauss cclauss force-pushed the remove_black_option_--diff_again branch from 28e42e4 to cd51b14 Compare June 21, 2022 12:20
@cclauss cclauss force-pushed the remove_black_option_--diff_again branch 3 times, most recently from 9bb4163 to 9205b8c Compare July 1, 2022 06:44
@cclauss
Copy link
Contributor Author

cclauss commented Jul 1, 2022

OK, Now this PR only modifies two files to make it easy to review and merge.

  1. .pre-commit-config.yaml: remove --diff so that all non-excluded files will be black formatted by pre-commit.ci
  2. pyproject.toml: add --force-exclude of specific files which we can black format in future PRs.

@cclauss cclauss force-pushed the remove_black_option_--diff_again branch from 9205b8c to 8b6a0d3 Compare July 8, 2022 09:13
@cclauss cclauss force-pushed the remove_black_option_--diff_again branch from 375864f to 20a5d8b Compare July 8, 2022 09:22
@cclauss cclauss merged commit 3738c38 into internetarchive:master Jul 9, 2022
@cclauss cclauss deleted the remove_black_option_--diff_again branch July 9, 2022 15:44
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