Skip to content

Whitespace precommit - #3894

Open
jacomago wants to merge 2 commits into
ControlSystemStudio:masterfrom
jacomago:whitespace-precommit
Open

Whitespace precommit#3894
jacomago wants to merge 2 commits into
ControlSystemStudio:masterfrom
jacomago:whitespace-precommit

Conversation

@jacomago

Copy link
Copy Markdown
Contributor

Checklist

  • Testing:

    • The feature has automated tests
    • Tests were run
    • If not, explain how you tested your changes
  • Documentation:

    • The feature is documented
    • The documentation is up to date
    • Release notes:
      • Added an entry if the change is breaking or significant
      • Added an entry when adding a new feature

@sonarqubecloud

Copy link
Copy Markdown

@lcaouen lcaouen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hard to review all the files but as it's only removing extra white space, there is no risk to approve it.

@shroffk

shroffk commented Jul 24, 2026

Copy link
Copy Markdown
Member

Oh wow...This is a lot

I think that we can configure this check such that we apply it only when we are going to be committing changes to a file. This way we don't have a 1k file changeset and the git history shows everything was last modified by this one commit.

We had followed similar rule with other repos were we applied a new linter... we would only apply it to the files that were part of a new PR

I thought that the above workflow was the default pre-commit behaviour...

@kasemir

kasemir commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

.. apply it only when we are going to be committing changes to a file

Applying this whitespace change to the complete code base risks creating merge conflicts in pending PRs.
Better fix whitespace whenever a module undergoes a functional change.

@shroffk

shroffk commented Jul 27, 2026

Copy link
Copy Markdown
Member

can we break this up

  1. PR with the pre-commit config
  2. PR fixing all the documentation files
  3. then we apply the fix with each PR and the files it touches

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