Skip to content

Conversation

anishk85
Copy link
Contributor

@anishk85 anishk85 commented Oct 5, 2025

Pull Request: Clarify Naming Convention Deviations in ROS 2 C++ Style Guide

What was fixed?

  • Added a clear notice in the C++ "Variable Naming" section of the documentation.
  • The notice explains that ROS 2 projects use a mix of snake_case, PascalCase, and UPPER_CASE for constants and variables, which deviates from the Google C++ Style Guide (which recommends kPascalCase for constants).
  • Rationale and guidance were provided for contributors:
    • This deviation is due to historical reasons and consistency with existing ROS codebases.
    • New projects should follow the conventions used in related ROS 2 packages.
    • When in doubt, contributors should prefer consistency with surrounding code over strict adherence to Google style.

Why was this needed?

  • There was confusion among contributors about the naming conventions, since the documentation referenced the Google C++ Style Guide but did not explain the deviations.
  • This update makes the documentation more transparent and helps maintain consistency across the codebase.

Where was it fixed?

  • The change was made in
    source/The-ROS2-Project/Contributing/Code-Style-Language-Versions.rst
    under the C++ > Variable Naming section.

Example of the new notice:

Note on naming conventions:
ROS 2 deviates from the Google C++ Style Guide in several naming areas:

  • The Google style guide recommends kPascalCase for constants (e.g., kDaysInAWeek)
  • ROS 2 projects currently use a mix of snake_case, PascalCase, and UPPER_CASE naming conventions
  • This deviation is for historical reasons and consistency with existing ROS codebases
  • For new projects, developers should follow the existing conventions in related ROS 2 packages
  • When in doubt, prefer consistency with surrounding code over strict adherence to Google style

This change improves clarity for all contributors and reviewers working on ROS 2 C++ code.

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

a few minor comments, basically lgtm but i would like to have another maintainer review before merge.

@anishk85
Copy link
Contributor Author

anishk85 commented Oct 6, 2025

@fujitatomoya okay so how does this looks to you

Note on naming conventions:

ROS 2 projects show inconsistent application of the Google C++ Style Guide for constants and naming:

  • The Google style guide recommends kPascalCase for constants (e.g., kDaysInAWeek)

  • Across ROS 2 packages, you'll find constants named using kPascalCase (e.g., in rosbag2_cpp),
    UPPER_CASE, PascalCase, and snake_case

  • Guidance for contributors:

    • For new code, follow the existing conventions in your specific package to maintain local consistency
    • When starting new packages or when conventions are unclear, prefer kPascalCase as it aligns
      with the Google C++ Style Guide
    • Reserve UPPER_CASE for macros only
    • Use snake_case for variables and functions (avoid PascalCase in these contexts)
    • When in doubt, prioritize consistency with surrounding code over strict adherence to any single convention

@fujitatomoya
Copy link
Collaborator

@anishk85 thanks for the reply.

The Google style guide recommends kPascalCase for constants (e.g., kDaysInAWeek)

i would remove this, because it is just a recommendation in Google code style that has been said in the top of this section already. probably we do not re-explain the recommendation of Google code style.

other than that, i am okay with it. but i would like to have another review from maintainers before merge.

@anishk85
Copy link
Contributor Author

@fujitatomoya can you tag another reviewer to kindly review this as soon as possible

@fujitatomoya
Copy link
Collaborator

@christophebedard @ahcorde can you take a look at this when you have time?

@ahcorde ahcorde added the backport-all backport at reviewers discretion; from rolling to all versions label Oct 13, 2025
@ahcorde ahcorde merged commit 8813cbc into ros2:rolling Oct 13, 2025
5 checks passed
mergify bot pushed a commit that referenced this pull request Oct 13, 2025
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
Co-authored-by: Alejandro Hernández Cordero <[email protected]>
(cherry picked from commit 8813cbc)
mergify bot pushed a commit that referenced this pull request Oct 13, 2025
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
Co-authored-by: Alejandro Hernández Cordero <[email protected]>
(cherry picked from commit 8813cbc)
mergify bot pushed a commit that referenced this pull request Oct 13, 2025
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
Co-authored-by: Alejandro Hernández Cordero <[email protected]>
(cherry picked from commit 8813cbc)
ahcorde added a commit that referenced this pull request Oct 13, 2025
(cherry picked from commit 8813cbc)

Signed-off-by: Alejandro Hernández Cordero <[email protected]>
Co-authored-by: Anish Kumar <[email protected]>
Co-authored-by: Alejandro Hernández Cordero <[email protected]>
ahcorde added a commit that referenced this pull request Oct 13, 2025
(cherry picked from commit 8813cbc)

Signed-off-by: Alejandro Hernández Cordero <[email protected]>
Co-authored-by: Anish Kumar <[email protected]>
Co-authored-by: Alejandro Hernández Cordero <[email protected]>
ahcorde added a commit that referenced this pull request Oct 13, 2025
(cherry picked from commit 8813cbc)

Signed-off-by: Alejandro Hernández Cordero <[email protected]>
Co-authored-by: Anish Kumar <[email protected]>
Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-all backport at reviewers discretion; from rolling to all versions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants