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

Initial clang-tidy configuration #1253

Merged
merged 10 commits into from
Sep 24, 2024
Merged

Initial clang-tidy configuration #1253

merged 10 commits into from
Sep 24, 2024

Conversation

farmerpiki
Copy link
Contributor

This adds some configuration to clang tidy, more to come in the future as well as more fixes to the code to go along with it.

Thanks.

@hsutter
Copy link
Owner

hsutter commented Aug 21, 2024

Thanks for your pull request! It looks like this may be your first contribution to cppfront. I've emailed you the Contributor License Agreement (CLA), and once it's signed I can look at your pull request. Thanks again for your contribution.

@farmerpiki
Copy link
Contributor Author

Signed.

@hsutter
Copy link
Owner

hsutter commented Aug 22, 2024

Got it, thanks Radu!

source/common.h Outdated Show resolved Hide resolved
source/common.h Outdated Show resolved Hide resolved
source/cppfront.cpp Outdated Show resolved Hide resolved
source/reflect.h Outdated Show resolved Hide resolved
source/reflect.h2 Outdated Show resolved Hide resolved
Copy link
Owner

@hsutter hsutter left a comment

Choose a reason for hiding this comment

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

Thanks! Here are some comments, and I've already gone and applied them myself.

source/common.h Outdated Show resolved Hide resolved
source/common.h Show resolved Hide resolved
source/cppfront.cpp Outdated Show resolved Hide resolved
source/reflect.h2 Show resolved Hide resolved
source/reflect.h2 Outdated Show resolved Hide resolved
source/reflect.h2 Show resolved Hide resolved
source/to_cpp1.h Outdated Show resolved Hide resolved
source/to_cpp1.h Show resolved Hide resolved
source/reflect.h2 Outdated Show resolved Hide resolved
@hsutter
Copy link
Owner

hsutter commented Sep 3, 2024

Looks good, we just have the reflect.h merge conflict... usually the line numbers changes don't cause a conflict, and maybe the simplest workaround is to move the reflect.h/h2 changes to a separate PR?

@farmerpiki
Copy link
Contributor Author

Looks good, we just have the reflect.h merge conflict... usually the line numbers changes don't cause a conflict, and maybe the simplest workaround is to move the reflect.h/h2 changes to a separate PR?

I hid the initialization thing for now cause some didn't like the immediately invoked lambda expression..., without that the line number changes are no longer an issue

I think I'll leave it until you relax the rules for out parameters.

@farmerpiki
Copy link
Contributor Author

how can I get those workflows approved?

@gregmarr
Copy link
Contributor

I'm sure Herb is busy with CppCon right now. The conference opens tomorrow and he has the opening keynote on Monday morning.

@hsutter
Copy link
Owner

hsutter commented Sep 24, 2024

Thanks!

@hsutter hsutter merged commit 0f10453 into hsutter:main Sep 24, 2024
21 of 29 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