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

feat: add black GHA for changed files #1356

Merged
merged 9 commits into from
Jul 20, 2023
Merged

Conversation

ahuber21
Copy link
Contributor

Provide a github action that runs black code formatting in PRs.
I'm trying to configure it such that only changed files are checked.

@ahuber21 ahuber21 marked this pull request as draft July 11, 2023 18:43
@ahuber21 ahuber21 force-pushed the dev/ahuber/black branch 16 times, most recently from a6c5366 to 1c49d78 Compare July 12, 2023 12:00
@ahuber21 ahuber21 marked this pull request as ready for review July 12, 2023 12:13
@ahuber21 ahuber21 force-pushed the dev/ahuber/black branch 2 times, most recently from faf3f86 to 6661df6 Compare July 14, 2023 14:03
@ahuber21 ahuber21 requested a review from napetrov July 17, 2023 18:13
setup.py Outdated Show resolved Hide resolved
@samir-nasibli
Copy link
Contributor

Thanks @ahuber21! Looks good to me. @napetrov @Alexsandruss could you please review?

Copy link
Contributor

@Alexsandruss Alexsandruss left a comment

Choose a reason for hiding this comment

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

It's not clear how black will affect code: for example, it changed license header when I manually applied it.
Using of it for only changed files will cause formatting inconsistency between changed part and old part of repo.

@ahuber21
Copy link
Contributor Author

ahuber21 commented Jul 19, 2023

Well it reformats the license because it's not pep8 conform

Block comments generally apply to some (or all) code that follows them, and are indented to the same level as that code. Each line of a block comment starts with a # and a single space (unless it is indented text inside the comment).

Therefore black adds the missing whitespace.

The effect of black is well defined and according to the docs, the coding style "can be viewed as a strict subset of PEP 8."
https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html

We already have many style inconsistencies. I would prefer conserving a reasonable commit history for files that do not change often which is why I'm adding black only for changed files. I think the benefits outweigh the downsides (like more inconsistencies). Frequently used/changed files will quickly be aligned to the consistent coding style.

@ahuber21
Copy link
Contributor Author

/intelci: run

@Alexsandruss
Copy link
Contributor

/intelci: run

@ahuber21 ahuber21 merged commit 1fc8e54 into intel:master Jul 20, 2023
@ahuber21 ahuber21 deleted the dev/ahuber/black branch July 20, 2023 16:41
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.

4 participants