fix(c++): Add requirement for section title comments above each section of a class declaration.#53
Merged
davidlion merged 11 commits intoy-scope:mainfrom Mar 27, 2026
Merged
Conversation
…s declaration ordering.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRestructures the C++ contribution guide's class member declaration order: replaces the previous single-block ordering with explicit per-access-specifier sections titled and ordered as Types; Static constants; Factory methods; Static methods; Static data members; Constructors; Operators; Destructor; Methods implementing ; Methods overriding ; Methods; Data members. Adds a prefatory clarification and reference. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/dev-guide/contrib-guides-cpp.md`:
- Line 135: The reference link for "Google's style guide" uses a malformed
target name (google-cpp-style-declaration-order) causing the reference added
later to be treated as unused; update the inline link to use proper
reference-link syntax by replacing the current malformed target with a valid
reference-style link that matches the reference identifier used at the bottom of
the document (ensure the link text "Google's style guide" points to
[google-cpp-style-declaration-order] so the later reference is recognized).
3 tasks
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/dev-guide/contrib-guides-cpp.md`:
- Line 137: Edit the sentence "Under every existing access specifier of a class
we require that declarations are organized into" by inserting a comma after the
word "class" so it reads "Under every existing access specifier of a class, we
require that declarations are organized into"; update the doc line containing
that exact phrase in contrib-guides-cpp.md.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/dev-guide/contrib-guides-cpp.md (1)
137-137:⚠️ Potential issue | 🟡 MinorAdd comma after "class" for improved readability.
The introductory phrase should be separated from the main clause with a comma.
📝 Proposed fix
-Under every existing access specifier of a class we require that declarations are organized into +Under every existing access specifier of a class, we require that declarations are organized into🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/dev-guide/contrib-guides-cpp.md` at line 137, In the sentence that begins "Under every existing access specifier of a class we require that declarations are organized into" add a comma after the word "class" so it reads "Under every existing access specifier of a class, we require that declarations are organized into" to separate the introductory phrase from the main clause.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/dev-guide/contrib-guides-cpp.md`:
- Around line 153-156: Update the two headings/descriptions "Methods
implementing InheritedClass" and "Methods overriding InheritedClass" to make
clear that "InheritedClass" is a placeholder: add a short parenthetical note or
sentence (near the headings or immediately after) instructing readers to replace
InheritedClass with the actual base/parent class name used in their code, and
optionally show a tiny example like "e.g., InheritedClass -> BaseWidget"; update
the text surrounding the symbols "Methods implementing InheritedClass" and
"Methods overriding InheritedClass" to include this clarification.
---
Duplicate comments:
In `@docs/dev-guide/contrib-guides-cpp.md`:
- Line 137: In the sentence that begins "Under every existing access specifier
of a class we require that declarations are organized into" add a comma after
the word "class" so it reads "Under every existing access specifier of a class,
we require that declarations are organized into" to separate the introductory
phrase from the main clause.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/dev-guide/contrib-guides-cpp.md`:
- Around line 153-161: Replace raw angle-bracket placeholder occurrences of
<InheritedClass> with an escaped or code-formatted form to prevent Markdown from
treating it as HTML: update the list items "Methods implementing
<InheritedClass>" and "Methods overriding <InheritedClass>" and the final note
so they use backticks (`<InheritedClass>`) or HTML escapes
(<InheritedClass>) instead of raw brackets; locate the strings "Methods
implementing <InheritedClass>", "Methods overriding <InheritedClass>", and the
line "Note, <InheritedClass> is a placeholder..." and change each instance
accordingly.
ℹ️ Review info
Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 95395440-3883-4232-a9a9-fda19b68b040
📒 Files selected for processing (1)
docs/dev-guide/contrib-guides-cpp.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/dev-guide/contrib-guides-cpp.md`:
- Around line 142-158: The ordered list has inconsistent nesting causing
markdownlint MD029; locate the list items "Types", "Factory methods", "Methods
implementing `InheritedClass`", and "Methods overriding `InheritedClass`" and
fix their nested bullets so the list continuity is preserved: either flatten the
nested lines into the same top-level list or convert the child lines into
properly indented sub-lists using a consistent 4-space indent (or consistent
two-space style used elsewhere) and a correct list marker, ensuring each
sub-item (e.g., "`typedef`, `using`, `enum`", "Declarations of static methods
that return an object of the class being defined.", "Declarations of methods
implementing abstract methods from the class `InheritedClass`.", "Declarations
of methods overriding virtual methods from the class `InheritedClass`.") is
formatted as a valid sub-list entry to avoid breaking the ordered-list sequence.
ℹ️ Review info
Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 869ab365-6ddd-4114-9569-1bde1f1338ac
📒 Files selected for processing (1)
docs/dev-guide/contrib-guides-cpp.md
Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com>
Member
kirkrodrigues
left a comment
kirkrodrigues
left a comment
There was a problem hiding this comment.
For the PR title, how about:
fix(c++): Add requirement for section title comments above each section of a class declaration.
Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com>
davidlion
changed the title
Comment on lines
157
to
+167
| [google-cpp-style-guide]: https://google.github.io/styleguide/cppguide.html | ||
| [google-cpp-style-guide-classes]: https://google.github.io/styleguide/cppguide.html#Classes | ||
| [google-cpp-style-declaration-order]: https://google.github.io/styleguide/cppguide.html#Declaration_Order |
Member
There was a problem hiding this comment.
Alphabetize:
Suggested change
| [google-cpp-style-guide]: https://google.github.io/styleguide/cppguide.html | |
| [google-cpp-style-guide-classes]: https://google.github.io/styleguide/cppguide.html#Classes | |
| [google-cpp-style-declaration-order]: https://google.github.io/styleguide/cppguide.html#Declaration_Order | |
| [google-cpp-style-declaration-order]: https://google.github.io/styleguide/cppguide.html#Declaration_Order | |
| [google-cpp-style-guide]: https://google.github.io/styleguide/cppguide.html | |
| [google-cpp-style-guide-classes]: https://google.github.io/styleguide/cppguide.html#Classes |
Arguably, it should be google-cpp-style-guide-declaration-order, or we drop -guide from the other links.
kirkrodrigues
approved these changes
Mar 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
As title says.
Checklist
breaking change.
Validation performed
CI passes. Declaration ordering and comments applied to some classes in CLP.
Summary by CodeRabbit