Skip to content

Trim trailing whitespaces and update pre-commit config to include "trailing-whitespace"#7332

Merged
jcfr merged 2 commits intoSlicer:mainfrom
jcfr:update-pre-commit-adding-trailing-whitespace-hook
Nov 2, 2023
Merged

Trim trailing whitespaces and update pre-commit config to include "trailing-whitespace"#7332
jcfr merged 2 commits intoSlicer:mainfrom
jcfr:update-pre-commit-adding-trailing-whitespace-hook

Conversation

@jcfr
Copy link
Member

@jcfr jcfr commented Nov 1, 2023

Consistently avoiding trailing whitespaces is helpful to avoid "false differences" and ensure all diffing tools show the "real" changes by default.

By adding trailing-whitespace to the pre-commit configuration, it enables the following:

  • GitHub Action report: Suggested changes are reported in the error log associated with the pre-commit GitHub Action workflow.

  • Consolidated local pre-commit checks and updates by running:

    PythonSlicer -m pip install pyupgrade
    
    PythonSlicer -m pre_commit run --all-files
    

Once this is integrated we will add the commit STYLE: Trim trailing whitespaces to .git-blame-ignore-revs1 as it was one:

Footnotes

  1. https://github.com/Slicer/Slicer/blob/main/.git-blame-ignore-revs

jcfr added 2 commits November 1, 2023 18:33
Consistently avoiding trailing whitespaces is helpful to avoid
"false differences" and ensure all diffing tools show the "real"
changes by default.
By adding `trailing-whitespace` to the pre-commit configuration, it enables the following:
* GitHub Action report: Suggested changes are reported in the error log associated with the pre-commit GitHub Action workflow.
* Consolidated local pre-commit checks and updates by running:

  ```
  PythonSlicer -m pip install pyupgrade

  PythonSlicer -m pre_commit run --all-files
  ```
Copy link
Contributor

@jamesobutler jamesobutler left a comment

Choose a reason for hiding this comment

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

My group uses this trailing whitespace check in our own work with success. Adding it here makes sense.

For additional reference we also use - id: check-ast

@jcfr jcfr merged commit 1886057 into Slicer:main Nov 2, 2023
@jcfr jcfr deleted the update-pre-commit-adding-trailing-whitespace-hook branch November 2, 2023 01:20
@jcfr
Copy link
Member Author

jcfr commented Nov 2, 2023

Thanks for the feedback. I will gradually introduce new checks.

@jcfr jcfr added the backport:5.x Identify pull request expected to be backported to the current 5.x release branch. label Nov 5, 2023
@jcfr jcfr removed the backport:5.x Identify pull request expected to be backported to the current 5.x release branch. label Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants