-
Notifications
You must be signed in to change notification settings - Fork 17.6k
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
Tools: Recommend what to do when astyle fails #28557
Tools: Recommend what to do when astyle fails #28557
Conversation
Tools/scripts/run_astyle.py
Outdated
@@ -60,7 +60,7 @@ def check(self): | |||
self.progress("astyle check failed: (%s)" % (ret.stdout)) | |||
self.retcode = 1 | |||
if "Formatted" in ret.stdout: | |||
self.progress("Files needing formatting found") | |||
self.progress("Files needing formatting found. Please run ./Tools/scripts/run_astyle.py") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really strange thing to be printing in run_astyle.py
!
If this is intended to make CI clearer, grepping the output of the dry-run might be a better idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is your expectation that we only print this in build_ci.sh
? I don't see why we need to grep when we can just check the return code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's my expectation - don't confuse the user who is running this manually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please look again. I've moved this message to build-ci.sh and check the error code.
Signed-off-by: Ryan Friedman <[email protected]>
Signed-off-by: Ryan Friedman <[email protected]>
30e2750
to
bb56e9c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Purpose
Tell developers what to do if the astyle check fails.
Motivation
ArduPilot/ardupilot_wiki#6292 (comment)
Demo
This is what a developer will see if they push code and CI fails.
Specifically, we went from:
to:
In the screenshot below, I intentionally broke formatting in AP_DDS_Client.h. This is what the script will display in CI.