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

Code formatting using black (infra) #1163

Merged
merged 15 commits into from
Apr 11, 2024
Merged

Code formatting using black (infra) #1163

merged 15 commits into from
Apr 11, 2024

Conversation

pieqq
Copy link
Collaborator

@pieqq pieqq commented Apr 9, 2024

Description

  • Format all the Python code in the monorepo using black, with line-length set to 79
  • Add a GitHub Action to ensure all new code comply with this rule
  • Set line-length option to the different sub-projects so that running black . or black . --diff locally from within one of these projects (checkbox-ng, the base provider, etc.) runs what's expected from the CI
  • Document this in the contributing guide
  • Add the commits related to code formatting to a .git-blame-ignore-revs so that they are ignored when browsing GitHub, or when using git config blame.ignoreRevsFile .git-blame-ignore-revs (as explained in the contrib guide)

Resolved issues

CHECKBOX-784

Documentation

Contributing guide is updated to include some information about black as code formatter.

Tests

Copy link

codecov bot commented Apr 9, 2024

Codecov Report

Attention: Patch coverage is 48.76126% with 455 lines in your changes are missing coverage. Please review.

Project coverage is 43.15%. Comparing base (961183e) to head (7745ec3).

Files Patch % Lines
checkbox-ng/plainbox/impl/exporter/xlsx.py 3.54% 135 Missing and 1 partial ⚠️
checkbox-ng/plainbox/impl/execution.py 3.12% 60 Missing and 2 partials ⚠️
checkbox-ng/plainbox/impl/session/restart.py 26.66% 42 Missing and 2 partials ⚠️
checkbox-ng/plainbox/impl/secure/config.py 28.00% 16 Missing and 2 partials ⚠️
checkbox-ng/plainbox/impl/commands/inv_session.py 0.00% 15 Missing ⚠️
checkbox-ng/plainbox/impl/_argparse.py 0.00% 14 Missing ⚠️
checkbox-ng/checkbox_ng/launcher/merge_reports.py 59.37% 10 Missing and 3 partials ⚠️
checkbox-ng/plainbox/impl/exporter/tar.py 0.00% 13 Missing ⚠️
checkbox-ng/plainbox/impl/secure/providers/v1.py 71.11% 7 Missing and 6 partials ⚠️
checkbox-ng/plainbox/impl/clitools.py 25.00% 11 Missing and 1 partial ⚠️
... and 34 more
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1163   +/-   ##
=======================================
  Coverage   43.14%   43.15%           
=======================================
  Files         355      355           
  Lines       38624    38627    +3     
  Branches     6559     6556    -3     
=======================================
+ Hits        16665    16668    +3     
  Misses      21294    21294           
  Partials      665      665           
Flag Coverage Δ
checkbox-ng 67.54% <48.64%> (+<0.01%) ⬆️
checkbox-support 52.14% <ø> (ø)
provider-base 16.11% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pieqq pieqq force-pushed the CHECKBOX-784-black-formatting branch 3 times, most recently from 7906c65 to ac46f1d Compare April 11, 2024 00:47
pieqq added 14 commits April 11, 2024 15:32
black uses pyproject.toml to store its configuration. Update the
different projects in the monorepo to match line length of 79 when
calling black.

See black documentation[1] for more information.

[1]
https://black.readthedocs.io/en/stable/usage_and_configuration/the_basics.html#configuration-via-a-file
Command used:

    black . --line-length 79 --extend-exclude "/vendor/"
Command used:

    black . --line-length 79 --extend-exclude "/vendor/"
Command used:

    black . --line-length 79 --extend-exclude "/vendor/"
Command used:

    black . --line-length 79 --extend-exclude "/vendor/"
Command used:

    black . --line-length 79 --extend-exclude "/vendor/"
Command used:

    black . --line-length 79 --extend-exclude "/vendor/"
As explained in this issue[1], E203[2] is not PEP 8 compliant so it
should be ignored when running flake8.

Black is now being used in the codebase, and it takes care of the
formatting.

[1] psf/black#315
[2] https://www.flake8rules.com/rules/E203.html
Although Black is configured at 79 chars line length, it keeps a few
lines to 80, 81 or 82 chars. Flake8 will complain about this, but we can
ignore these.
@pieqq pieqq force-pushed the CHECKBOX-784-black-formatting branch from ac46f1d to e77ab97 Compare April 11, 2024 07:35
Hook25
Hook25 previously approved these changes Apr 11, 2024
Waiting until the squashed commit gets merged, and will create one
afterwards.
@pieqq pieqq merged commit 3789fdd into main Apr 11, 2024
30 of 31 checks passed
@pieqq pieqq deleted the CHECKBOX-784-black-formatting branch April 11, 2024 08:43
LiaoU3 pushed a commit to LiaoU3/checkbox that referenced this pull request Apr 17, 2024
* Set line length to 79 when calling black

black uses pyproject.toml to store its configuration. Update the
different projects in the monorepo to match line length of 79 when
calling black.

See black documentation[1] for more information.

[1]
https://black.readthedocs.io/en/stable/usage_and_configuration/the_basics.html#configuration-via-a-file

* Update contributing guide to mention black

* Add black GitHub Action

* Ignore vendorized code when running black

* Format all the codebase using black

Command used:

    black . --line-length 79 --extend-exclude "/vendor/"

* Ignore E203 (Whitespace before ':')

As explained in this issue[1], E203[2] is not PEP 8 compliant so it
should be ignored when running flake8.

Black is now being used in the codebase, and it takes care of the
formatting.

[1] psf/black#315
[2] https://www.flake8rules.com/rules/E203.html

* Ignore flake8 complaints for a few specific lines in the base provider

Although Black is configured at 79 chars line length, it keeps a few
lines to 80, 81 or 82 chars. Flake8 will complain about this, but we can
ignore these.
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