Run pre-commit for ArduPilot source automatically

[Ryanf55]

AP_DDS requires pre-commit to be run. If you don't have it set up, you have to run it and squash the changes. CI could actually support changing the files automatically.

Last time we discussed this in dev call for how astyle isn't applied automatically, I suggested pre-commit, however that was shot down because astyle is not available on all developer machines. With pre-commit auto-fixing the files in CI, this is no longer a concern, and could be adopted more widely across the repo.

          YES!!!! Except it's really a global thing that doesn't belong inside AP_DDS. Ideally we'd have some suggestions to use tools like these when creating a PR.... as I suggested to @peterbarker during his tools talk at the recent Dev conference.

https://youtu.be/-5JoR29ZlrA?t=1738

Originally posted by @magicrub in https://github.com/ArduPilot/ardupilot/issues/23535#issuecomment-1512330756

[Ryanf55]

Looks theres' some conflicts with Tools/gittools/pre-commit.py which overwrites pre-commit.

(venv-ardupilot) ryan@ryan-B650-970:~/Development/ardu_ws/src/ardupilot$ pre-commit install
Running in migration mode with existing hooks at .git/hooks/pre-commit.legacy
Use -f to use only pre-commit.
pre-commit installed at .git/hooks/pre-commit

Additionally, pre-commit doesn't run without errors right now. It's not enforced in CI, so there will be regressions.

To start, we can add exceptions for all the current regressions, and enforce no new regressions in CI.

[Ryanf55]

Would be nice to run astyle in CI too to prevent errors like these: https://github.com/ArduPilot/ardupilot/pull/24792#discussion_r1307250568

[Ryanf55]

Asks from Tridge:

• We want it used in Github
• We need to ensure the code formatter doesn't cause parsing parameters
• We should use git-blame-ignore-revs to preserve history if we retroactively apply formatting
• We need to make it as easy as possible for people using VSCode - is that enough?
• It is mostly a good idea to format new libraries
• We don't expect people to format partial files
• The tools should be low enough friction to save us time in the code (the tools get better over time, eventually it will be worth it)
• We need a way to ignore it
• If we apply server side hooks, how do we protect from making them force reset
• We must balance the input of new developers, this is critical to the success of the project
• If there was a button in github to make stuff merge on CI pass to adjust code style that they have added, that would be useful

Random - we want force push to remove merge on CI pass unless you have merge rights. The commit per subsystem needs to do more than just the head. The python git API's could do it.

[khancyr]

• We want it used in Github [ X ] That is possible and quite easy

• We need to ensure the code formatter doesn't cause parsing parameters

• We should use git-blame-ignore-revs to preserve history if we retroactively apply formatting

• We need to make it as easy as possible for people using VSCode - is that enough? [ X ] normally that is well integrated, as we already require python we shouldn't have issue with pre-commit

• It is mostly a good idea to format new libraries

• We don't expect people to format partial files [ ] that could cause issue on start as precommit will try to fix modified files as people are making change

• The tools should be low enough friction to save us time in the code (the tools get better over time, eventually it will be worth it)

• We need a way to ignore it [ X ] We just need to document it, it is easy to disable and force no check on cmdline, not sure on IDE

• If we apply server side hooks, how do we protect from making them force reset Do we want to fix happens on Github ? or just a report to use the tools to fix localy and update commits/PR ?

• We must balance the input of new developers, this is critical to the success of the project

• If there was a button in github to make stuff merge on CI pass to adjust code style that they have added, that would be useful

[Ryanf55]

• We don't expect people to format partial files [ ] that could cause issue on start as precommit will try to fix modified files as people are making change

pre-commit only runs when you commit, which means you are done making changes and have typed git commit.

[Ryanf55]

• We need a way to ignore it [ X ] We just need to document it, it is easy to disable and force no check on cmdline, not sure on IDE

I need to know how people use git in an IDE without CLI. Can anyone share what tools they use or that new contributors use that aren't CLI?

[timtuxworth]

Just go with whatever VSCode does. I use CLI, but I think most IDE people are using VSCode.

[Ryanf55]

Just go with whatever VSCode does. I use CLI, but I think most IDE people are using VSCode.

There is nothing built in to the default vscode that handles C++ formatting. It's all through extensions.

[cclauss]

These commands all currently fail on localhost (a developer's machine): ~% pre-commit run --all-files check-executables-have-shebangs~ Fixed in #30359 (merged) ~% pre-commit run --all-files check-shebang-scripts-are-executable~ Fixed in #30351 (merged) % pre-commit run --all-files flake8

[hendjoshsr71]

I use gitkraken and it’s associated tools like gitlens. I also use the "Git History" to show how files and lines have changed along with the associated files that changed in the same commit.

On Mon, Nov 18, 2024 at 18:34 Ryan @.***> wrote:

Just go with whatever VSCode does. I use CLI, but I think most IDE people are using VSCode.

There is nothing built in to the default vscode that handles C++ formatting. It's all through extensions.

— Reply to this email directly, view it on GitHub https://github.com/ArduPilot/ardupilot/issues/24555#issuecomment-2484387362, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQQEX5PE3K2RPUUG5TM3VUL2BJ2P3AVCNFSM6AAAAABR5NESI6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOBUGM4DOMZWGI . You are receiving this because you are subscribed to this thread.Message ID: @.***>