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

Tidy ups to variable and argument names #656

Merged
merged 11 commits into from
Aug 14, 2024
Merged

Conversation

jhol
Copy link
Contributor

@jhol jhol commented Jul 19, 2024

This patch-set makes all variables and arguments consistent with lower_case snake_case style, with a underscore prefix applied to local variables.

@valgur
Copy link
Contributor

valgur commented Jul 19, 2024

The _ variable prefixes are unnecessary in function-s, since their scope is limited only to the function. I would keep them only for variables created inside macro-s, which do have a global scope.

@jhol
Copy link
Contributor Author

jhol commented Jul 19, 2024

The _ variable prefixes are unnecessary in function-s, since their scope is limited only to the function. I would keep them only for variables created inside macro-s, which do have a global scope.

Ok, no _ variable prefixes in functions now, except where they are used as a shadows of output variables.

@jhol
Copy link
Contributor Author

jhol commented Jul 31, 2024

Can we have this merged?

@memsharded
Copy link
Member

Hi @jhol

The CI is red, this cannot be merged until it is green, could you please have a look?

@jhol
Copy link
Contributor Author

jhol commented Jul 31, 2024

Hi @jhol

The CI is red, this cannot be merged until it is green, could you please have a look?

My mistake. I thought the test system was non-functional for this branch. The build issues should be fixed now.

@jhol
Copy link
Contributor Author

jhol commented Aug 9, 2024

This should be ready for merge now. Is there anything I can do to help get this accepted?

Copy link
Member

@memsharded memsharded 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 to me, thanks for the contribution.

@memsharded memsharded added this to the cmake-conan2-0.1 milestone Aug 11, 2024
@jcar87 jcar87 merged commit c22bbf0 into conan-io:develop2 Aug 14, 2024
9 checks passed
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