Skip to content

Remove trailing whitespace #2791

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

Merged
merged 1 commit into from
Apr 4, 2025
Merged

Conversation

leander-dsouza
Copy link
Contributor

Description

I have added the following hooks as part of the pre-commit:

  • end-of-file-fixer
  • mixed-line-ending
  • trailing-whitespace

This prevents empty whitespace and additional newlines from registering as a separate commit during development.

@leander-dsouza leander-dsouza marked this pull request as ready for review March 31, 2025 16:00
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

please see my comment on ros2/ros2_documentation#5203 (review)

@christophebedard
Copy link
Member

This is almost certainly going to change existing files, right? Also, note that, given that we have a ton of repos, it would take a non-trivial amount of work to maintain this.

@fujitatomoya
Copy link
Collaborator

if we enable this globally, yeah we need to to make sure all repos to pass the github action with pre-commit, that is the work. (my thought is that keep the github action under ros2/.github repository as implementation, and use it to each repo one by one gradually to add the workflow file. so it will not be giant global switch ON/OFF i think.) basically what pre-commit does is right things, we can see that from this PR.

@nuclearsandwich
Copy link
Member

@ros-pull-request-builder retest this please

1 similar comment
@nuclearsandwich
Copy link
Member

@ros-pull-request-builder retest this please

@leander-dsouza
Copy link
Contributor Author

Should I close this PR and open a new one in the ros2 repository, targeting each package individually?

@fujitatomoya
Copy link
Collaborator

@leander-dsouza how would you want to process this? i think we can remove pre-commit thing from here, and merge the rest? Or if you want to close this, that is also fine.

Signed-off-by: Leander Stephen D'Souza <[email protected]>
@leander-dsouza leander-dsouza changed the title Add pre commit Remove trailing whitespace Apr 3, 2025
@leander-dsouza
Copy link
Contributor Author

leander-dsouza commented Apr 3, 2025

@fujitatomoya I have refactored the PR to remove the trailing whitespace :)

@fujitatomoya
Copy link
Collaborator

Pulls: #2791
Gist: https://gist.githubusercontent.com/fujitatomoya/2e3e7616c2133ca52f1a21f75fdb3b63/raw/67286d237758d2aa3c2ee4a33c48a192e7cfc509/ros2.repos
BUILD args: --packages-up-to rclcpp rclcpp_components
TEST args: --packages-select rclcpp rclcpp_components
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15577

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@fujitatomoya fujitatomoya merged commit 011b878 into ros2:rolling Apr 4, 2025
2 of 3 checks passed
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