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

pre-commit: Fix current errors #24727

Merged
merged 9 commits into from
Sep 8, 2023

Conversation

Ryanf55
Copy link
Collaborator

@Ryanf55 Ryanf55 commented Aug 22, 2023

This fixes all current errors with pre-commit in ArduPilot. Relates to #24683

Mainly:

  • Line endings
  • Wrong executable permissions
  • Missing shebang
  • Having a shebang without executable permissions
  • Invalid XML (changed to .j2 extension which is common for jinja templates)

image

@Ryanf55 Ryanf55 changed the title Pre commit fix current errors pre-commit: Fix current errors Aug 22, 2023
Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

Those commit messages are... not good.

Only one of them actually says what changed about the file, the others are "some tool told me to do this".

In the case of the line-endings changes, the fact that's not called out is a pretty big problem as the change is pretty bad as it is a significant change for the purpose of backporting bugfixes.

Should be seeing commit messages like "XXX: add shebang line to script" and "YYY: remove executable permissions" and "ZZZ: rename non-xml scrimmage template away from .xml".

@Ryanf55 Ryanf55 force-pushed the pre-commit-fix-current-errors branch from 206666d to e4f10a7 Compare August 23, 2023 01:07
@Ryanf55
Copy link
Collaborator Author

Ryanf55 commented Aug 23, 2023

Those commit messages are... not good.

Only one of them actually says what changed about the file, the others are "some tool told me to do this".

In the case of the line-endings changes, the fact that's not called out is a pretty big problem as the change is pretty bad as it is a significant change for the purpose of backporting bugfixes.

Should be seeing commit messages like "XXX: add shebang line to script" and "YYY: remove executable permissions" and "ZZZ: rename non-xml scrimmage template away from .xml".

I changed all the commit messages to be much more descriptive. Makes sense. Tridge recommended we decided case-by-case on the line-ending ones. That said, it seems @khancyr already tried to change it before in 22dae61, but it's still triggering the pre-commit violation. Any idea why we need to do it again?

@tridge tridge dismissed peterbarker’s stale review September 6, 2023 07:31

approved on eu dev call

* This fixes an error in pre-commit's check-xml hook
  * Tools/autotest/template/scrimmage.xml: Failed to xml parse (Tools/autotest/template/scrimmage.xml:8:7: not well-formed (invalid token))
* Since it's a template file, it is not parseable as XML till after it's rendered by jinja

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
* Source files are not supposed to be executable

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
* Hex files should not be executable
* .txt files should not be executable
* The DDS test listener was supposed to be executable but was missing a shebang

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
* Fixes a pre-commit violation; the ioc files are not supposed to be executable

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
* This script was marked as executable but did not have a shebang

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
* The Lua scripts are not directly invoked - they have no shebang, so they shouldn't be executable

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
* This file had DOS line endings and ArduPilot uses UNIX endings
* This fixes a pre-commit violation

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
@peterbarker peterbarker merged commit 35d1721 into ArduPilot:master Sep 8, 2023
85 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants