Skip to content

Feat/ Espressif clang check and remove unwanted python traceback (IDF-11835) #52

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 2 commits into from
Mar 25, 2025

Conversation

mfialaf
Copy link
Collaborator

@mfialaf mfialaf commented Mar 4, 2025

  1. Verify that Espressif clang is present, to prevent possible usage of system clang.
  2. In case of error, raise SystemExit instead of RuntimeError to prevent unwanted Python traceback in output.

@mfialaf mfialaf self-assigned this Mar 4, 2025
@mfialaf mfialaf force-pushed the feat/esp_clang_check branch 3 times, most recently from 4f68206 to ba7cd98 Compare March 5, 2025 09:47
@mfialaf mfialaf changed the title Feat/esp clang check and remove unwanted python traceabck Feat/ Espressif clang check and remove unwanted python traceback Mar 5, 2025
@mfialaf mfialaf requested a review from dobairoland March 5, 2025 09:56
Copy link
Collaborator

@fhrbata fhrbata left a comment

Choose a reason for hiding this comment

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

LGTM, maybe checking for clang-tidy instead of clang would be more relevant, since IIUC it's the clang-tidy which is call under the hood. It's just nitpick, no need to change it. Thank you

Copy link
Member

@radimkarnis radimkarnis left a comment

Choose a reason for hiding this comment

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

LGTM

@dobairoland dobairoland changed the title Feat/ Espressif clang check and remove unwanted python traceback Feat/ Espressif clang check and remove unwanted python traceback (IDF-11835) Mar 5, 2025
@dobairoland
Copy link
Collaborator

@mfialaf Thank you. LGTM in general. Could you please classify these commits as fixes instead of features?

@mfialaf mfialaf force-pushed the feat/esp_clang_check branch from ba7cd98 to 4b28d41 Compare March 19, 2025 14:04
@mfialaf
Copy link
Collaborator Author

mfialaf commented Mar 19, 2025

@dobairoland @fhrbata @radimkarnis Thank you all for revirew!

I considered your comments and adjusted the code. PR should be ready.

Copy link
Collaborator

@dobairoland dobairoland left a comment

Choose a reason for hiding this comment

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

@mfialaf Could you please correct the typo in the commit title in traceback?

Also, why is Espressif before clang-tidy? clang-tidy is not developed by Espressif.

@mfialaf mfialaf force-pushed the feat/esp_clang_check branch from 4b28d41 to 1e86710 Compare March 20, 2025 08:06
@dobairoland dobairoland merged commit 2dc62e9 into master Mar 25, 2025
4 checks passed
@dobairoland dobairoland deleted the feat/esp_clang_check branch March 25, 2025 09:43
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