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

Reformat code using clang-format and add CI/CD checks #39

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

supcik
Copy link

@supcik supcik commented May 28, 2024

My goal with this contribution is to simplify the collaboration with contributors. So I added a .clang-format file with settings compatible with the IDF Style Guide. I also added a pre-commit hook to enforce the formatting rules.

Then I added a pre-commit hook to check the code with cppcheck. The tool discovered a couple of issues, so I fixed them.

Finally, I configured the Github actions to check the formatting and to run cppcheck on every commit. I also added a simple example and I use Github actions to compile the example with several architectures (currently esp32 end esp32-s3) and several IDF versions (currently v4.4.7, v5.1.4, and v5.2.1)

Note : This pull request addresses the issue #38.

@abobija abobija linked an issue May 28, 2024 that may be closed by this pull request
@supcik
Copy link
Author

supcik commented May 30, 2024

Hello @abobija. You assigned this PR to me. Do you expect me to do something?

@abobija
Copy link
Owner

abobija commented May 30, 2024

No. I'll review it on my side as soon as I find some free time and I'll merge it if everything is OK.

Copy link
Owner

@abobija abobija left a comment

Choose a reason for hiding this comment

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

Looks good and I've tested it on my side, just small comment on gh workflow

Comment on lines +4 to +5
pull_request:
push:
Copy link
Owner

@abobija abobija Jun 9, 2024

Choose a reason for hiding this comment

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

Please use pull_request_target instead of pull_request, it's safer. And remove push trigger (no need for it, because running workflow on pr is enough, what do you think?)

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.

Please, add a ".clang-format" file
2 participants