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

Add style for enums and prefer enum classes #5149

Merged
merged 1 commit into from Jun 5, 2023

Conversation

Ryanf55
Copy link
Contributor

@Ryanf55 Ryanf55 commented May 24, 2023

Recommend enum classes since they are commonly used. Provide a style recommendation for them.
ArduPilot/ardupilot#23877
ArduPilot/ardupilot#23830
https://github.com/ArduPilot/ardupilot/pull/23768/files#diff-ed061f26ab421c524f35bb9112b56bc1322892589f95b77e9497345eb48eafbe
https://github.com/ArduPilot/ardupilot/pull/23119/files#diff-a98f7c976f44f4a82c8c4882b638ca053d70a95f603861216630e20243689dd2

I did not cover the aspect of using #DEFINE to enable/disable enums, and to be sure to pin all the numbers so they don't change value when you change your build options. Seems too advanced for the style guide.

Copy link
Contributor

@rmackay9 rmackay9 left a comment

Choose a reason for hiding this comment

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

In the code we have a mix of camel case and underscore for the enum names but I think we are tending towards camel case so I think this looks right. @peterbarker you agree?

dev/source/docs/style-guide.rst Outdated Show resolved Hide resolved
@peterbarker
Copy link
Contributor

In the code we have a mix of camel case and underscore for the enum names but I think we are tending towards camel case so I think this looks right. @peterbarker you agree?

This is actually PascalCase - not camelCase :-) camelCase is the one that has the hump in the middle :-)

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
@tridge tridge merged commit 751a6c5 into ArduPilot:master Jun 5, 2023
4 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.

None yet

6 participants