refactor: S10.22 enum naming convention restoration (SCREAMING_SNAKE)#391
Conversation
Phase 1 of the enum naming convention restoration: convert the 24 SolidSyslogFacility enum constants from project-prefix-PascalCase (SolidSyslogFacility_Kern) to SCREAMING_SNAKE (SOLIDSYSLOG_FACILITY_KERN). Compound member names get snake-separated at CamelCase boundaries: AuthPriv → AUTH_PRIV. Trailing digits stay glued: Local0 → LOCAL0. The enum type itself (struct/enum tag SolidSyslogFacility) is unchanged — only its constants are renamed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 2 of the enum naming convention restoration: convert the 8 SolidSyslogSeverity enum constants from project-prefix-PascalCase to SCREAMING_SNAKE. No compound member names — straightforward rename. The enum type itself (SolidSyslogSeverity) is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ICY_* Phase 3: 3 constants — Oldest, Newest, Halt. DiscardPolicy is two words, so the type prefix becomes DISCARD_POLICY. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…AM_SEND_RESULT_* Phase 4: 3 constants — Sent, Failed, Oversize. DatagramSendResult is three words, so the type prefix becomes DATAGRAM_SEND_RESULT. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 5: 2 constants — Udp, Tcp. Final tagged-enum rename in this sweep; the type prefix becomes TRANSPORT_UDP / TRANSPORT_TCP. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves the project-wide inconsistency where multi-word class names inside SCREAMING macros were glued (SOLIDSYSLOG_CIRCULARBUFFER_*) while others used snake separation (SOLIDSYSLOG_STREAM_SENDER_SIZE, SOLIDSYSLOG_WINSOCK_TCP_STREAM_SIZE). Aligns every multi-word class segment with the snake-at-every-CamelCase-boundary rule established for S10.22 enum constants. Renames (11): BLOCKSTORE → BLOCK_STORE CIRCULARBUFFER → CIRCULAR_BUFFER (4 sub-names) FILEBLOCKDEVICE → FILE_BLOCK_DEVICE FREERTOSDATAGRAM/MUTEX/STATICRESOLVER/TCPSTREAM → FREE_RTOS_* (4) POSIXMUTEX → POSIX_MUTEX STDATOMICCOUNTER → STD_ATOMIC_COUNTER WINDOWSATOMICCOUNTER → WINDOWS_ATOMIC_COUNTER WINDOWSMUTEX → WINDOWS_MUTEX Single-word class segments (SOLIDSYSLOG_FORMATTER_STORAGE_SIZE, SOLIDSYSLOG_ADDRESS_SIZE, SOLIDSYSLOG_WINDOWS_FILE_SIZE, SOLIDSYSLOG_WINSOCK_TCP_STREAM_SIZE, etc.) are unchanged — already in the right shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collapses the two-rule enum-constant convention introduced by S10.07 + S10.12 into a single mechanically-enforceable rule: - All enum constants → SCREAMING_SNAKE. No exceptions for tagged vs anonymous, public vs TU-local. - Snake-separate at every CamelCase boundary in the source identifier; trailing digits stay glued (Local0 → LOCAL0). - Public sites carry the SOLIDSYSLOG_ prefix (enforced by review + cppcheck-misra 5.4, since clang-tidy can't distinguish public from TU-local enum constants). .clang-tidy: EnumConstantCase → UPPER_CASE only; dropped the EnumConstantPrefix + EnumConstantIgnoredRegexp escape. One rule, no exceptions. NAMING.md: replaced the "Anonymous-enum named-constant idiom" carve-out with a single "Enum constants" section. Quick-reference table updated. Historical S10.07/S10.12 split documented as superseded. The previous two-rule system relied on per-site judgement to decide whether a constant was "macro-like enough" to warrant SCREAMING vs class-scoped PascalCase. That judgement was drifting; S10.16 surfaced a third hybrid form (Class_DEFAULT_INSTANCE) that nobody had decided on. One rule, one tidy setting, no IgnoredRegexp escape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two files needed reformat after the enum rename: backslash-continuation column alignment in a CHECK_REPORTED_ERROR macro shifted because the content shifted (Tests/Support/ErrorHandlerFake.h), and one test-fixture initialiser exceeded the column limit and reflowed (Tests/SolidSyslogBlockStoreDrainOrderingTest.cpp). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR implements the S10.22 enum constant naming convention restoration by renaming all enum constants from mixed-case PascalCase (e.g., ChangesEnum constant naming restoration (S10.22)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/misra-conformance.md (1)
87-87: ⚡ Quick winUpdate stale enum-constant wording to match S10.22.
Line 87 still references the older
SolidSyslogClass_Constantframing and an unresolved anonymous-enum split, which now conflicts with the single SCREAMING_SNAKE convention adopted in this PR. A small wording update here would prevent policy drift across docs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/misra-conformance.md` at line 87, Update the enum-constant wording at the cited paragraph to reflect S10.22's single SCREAMING_SNAKE convention instead of the older SolidSyslogClass_Constant framing: remove the reference to the unresolved anonymous-enum split and state that constants like SOLIDSYSLOG_SEVERITY_EMERGENCY and SOLIDSYSLOG_ADDRESS_SIZE follow SCREAMING_SNAKE; note the two remediation options (treat anonymous-enum entries as macro-equivalents and tighten the .clang-tidy regex, or rename them to a class-prefixed PascalCase form) so the docs match the repo decision taken in this PR.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Tests/DatagramFake.h`:
- Line 19: Update the inaccurate header comment that currently states calls
beyond configured slots return SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT; change it
to correctly state they return SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED to match
the behavior implemented in SendTo and the related test logic (reference
symbols: DatagramFake, SendTo, SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED).
---
Nitpick comments:
In `@docs/misra-conformance.md`:
- Line 87: Update the enum-constant wording at the cited paragraph to reflect
S10.22's single SCREAMING_SNAKE convention instead of the older
SolidSyslogClass_Constant framing: remove the reference to the unresolved
anonymous-enum split and state that constants like
SOLIDSYSLOG_SEVERITY_EMERGENCY and SOLIDSYSLOG_ADDRESS_SIZE follow
SCREAMING_SNAKE; note the two remediation options (treat anonymous-enum entries
as macro-equivalents and tighten the .clang-tidy regex, or rename them to a
class-prefixed PascalCase form) so the docs match the repo decision taken in
this PR.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8b572121-5be3-400e-b843-6e6ad3b7a879
📒 Files selected for processing (66)
.clang-tidyBdd/Targets/FreeRtos/main.cBdd/Targets/Linux/BddTargetCommandLine.cBdd/Targets/Linux/main.cBdd/Targets/Windows/BddTargetWindows.cBdd/Targets/Windows/BddTargetWindowsCommandLine.cCore/Interface/SolidSyslogBlockStore.hCore/Interface/SolidSyslogCircularBuffer.hCore/Interface/SolidSyslogDatagram.hCore/Interface/SolidSyslogFileBlockDevice.hCore/Interface/SolidSyslogPrival.hCore/Interface/SolidSyslogTransport.hCore/Source/BlockSequence.cCore/Source/SolidSyslog.cCore/Source/SolidSyslogBlockStore.cCore/Source/SolidSyslogCircularBuffer.cCore/Source/SolidSyslogFileBlockDevice.cCore/Source/SolidSyslogMetaSd.cCore/Source/SolidSyslogStreamSender.cCore/Source/SolidSyslogUdpSender.cDEVLOG.mdPlatform/Atomics/Interface/SolidSyslogStdAtomicCounter.hPlatform/FreeRtos/Interface/SolidSyslogFreeRtosDatagram.hPlatform/FreeRtos/Interface/SolidSyslogFreeRtosMutex.hPlatform/FreeRtos/Interface/SolidSyslogFreeRtosStaticResolver.hPlatform/FreeRtos/Interface/SolidSyslogFreeRtosTcpStream.hPlatform/FreeRtos/Source/SolidSyslogFreeRtosDatagram.cPlatform/FreeRtos/Source/SolidSyslogFreeRtosMutex.cPlatform/FreeRtos/Source/SolidSyslogFreeRtosStaticResolver.cPlatform/FreeRtos/Source/SolidSyslogFreeRtosTcpStream.cPlatform/Posix/Interface/SolidSyslogPosixMutex.hPlatform/Posix/Source/SolidSyslogGetAddrInfoResolver.cPlatform/Posix/Source/SolidSyslogPosixDatagram.cPlatform/Posix/Source/SolidSyslogPosixMutex.cPlatform/Windows/Interface/SolidSyslogWindowsAtomicCounter.hPlatform/Windows/Interface/SolidSyslogWindowsMutex.hPlatform/Windows/Source/SolidSyslogWindowsMutex.cPlatform/Windows/Source/SolidSyslogWinsockDatagram.cPlatform/Windows/Source/SolidSyslogWinsockResolver.cTests/Bdd/Targets/BddTargetServiceThreadTest.cppTests/Bdd/Targets/BddTargetWindowsCommandLineTest.cppTests/BlockSequenceTest.cppTests/DatagramFake.cTests/DatagramFake.hTests/DatagramFakeTest.cppTests/FreeRtos/SolidSyslogFreeRtosDatagramTest.cppTests/FreeRtos/SolidSyslogFreeRtosStaticResolverTest.cppTests/SolidSyslogBlockStoreDrainOrderingTest.cppTests/SolidSyslogBlockStorePosixTest.cppTests/SolidSyslogBlockStoreTest.cppTests/SolidSyslogCircularBufferTest.cppTests/SolidSyslogErrorTest.cppTests/SolidSyslogGetAddrInfoResolverTest.cppTests/SolidSyslogMetaSdTest.cppTests/SolidSyslogPosixDatagramTest.cppTests/SolidSyslogPosixMessageQueueBufferTest.cppTests/SolidSyslogPrivalTest.cppTests/SolidSyslogTest.cppTests/SolidSyslogUdpSenderTest.cppTests/SolidSyslogWinsockDatagramTest.cppTests/SolidSyslogWinsockResolverTest.cppTests/Support/ErrorHandlerFake.cTests/Support/ErrorHandlerFake.hdocs/NAMING.mddocs/misra-conformance.mddocs/misra-deviations.md
|
|
||
| /* Per-call SendTo result (0-based). Calls beyond the configured slots | ||
| * return SolidSyslogDatagramSendResult_Sent by default. */ | ||
| * return SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT by default. */ |
There was a problem hiding this comment.
Fix incorrect default-result behavior in the header comment.
Line 19 is inaccurate: calls beyond configured slots return SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED (per SendTo), not ..._SENT.
Suggested comment fix
- /* Per-call SendTo result (0-based). Calls beyond the configured slots
- * return SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT by default. */
+ /* Per-call SendTo result (0-based). Calls beyond the configured slots
+ * return SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED by default. */📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * return SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT by default. */ | |
| /* Per-call SendTo result (0-based). Calls beyond the configured slots | |
| * return SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED by default. */ |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Tests/DatagramFake.h` at line 19, Update the inaccurate header comment that
currently states calls beyond configured slots return
SOLIDSYSLOG_DATAGRAM_SEND_RESULT_SENT; change it to correctly state they return
SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED to match the behavior implemented in
SendTo and the related test logic (reference symbols: DatagramFake, SendTo,
SOLIDSYSLOG_DATAGRAM_SEND_RESULT_FAILED).
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1119 passed, 🙈 2 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
Purpose
Restore a single, mechanically enforceable rule for enum constant naming. The S10.07-introduced
SolidSyslog<Class>_<Member>convention and the S10.12 SCREAMING_SNAKE carve-out for the anonymous-enum named-constant idiom together formed a two-class system that's not enforceable by clang-tidy — only per-site judgement separates the cases. By the S10.16 review the boundary was drifting (a third hybrid formClass_DEFAULT_INSTANCEwas about to be introduced). Collapse to one rule.Closes #390.
Change Description
Single rule, mechanically enforced: all enum constants are
SCREAMING_SNAKE. Tagged or anonymous, public or TU-local.DatagramSendResult→DATAGRAM_SEND_RESULT,AuthPriv→AUTH_PRIV). Trailing digits stay glued (Local0→LOCAL0, matching POSIXLOG_LOCAL0).SOLIDSYSLOG_on public sites; bare SCREAMING on TU-local anonymous-enum constants (IPV4_HEADER_BYTES)..clang-tidyEnumConstantCase→UPPER_CASEonly; droppedEnumConstantPrefix+EnumConstantIgnoredRegexp.NAMING.md"Enum constants" section replaces the old two-section split. Quick-ref table updated.Sweep in 8 commits, build green between each:
SolidSyslogFacility_*→SOLIDSYSLOG_FACILITY_*(24 constants)SolidSyslogSeverity_*→SOLIDSYSLOG_SEVERITY_*(8)SolidSyslogDiscardPolicy_*→SOLIDSYSLOG_DISCARD_POLICY_*(3)SolidSyslogDatagramSendResult_*→SOLIDSYSLOG_DATAGRAM_SEND_RESULT_*(3)SolidSyslogTransport_*→SOLIDSYSLOG_TRANSPORT_*(2)CIRCULARBUFFER→CIRCULAR_BUFFER,STDATOMICCOUNTER→STD_ATOMIC_COUNTER, etc.) — pre-existing class-segment-glued macros didn't match the rule we were adopting; second pass aligns them..clang-tidy+NAMING.mdsingle-rule update.Static-variable naming (the
instance/DEFAULT_INSTANCEfamily) is explicitly out of scope — those are objects, not enum constants. The per-group conformance stories (S10.16 onwards) will hit them on touch.Test Evidence
debug+clang-debug+sanitizepresets (verified after every sweep commit).analyze-tidyclean — zero enum-case warnings under the newUPPER_CASErule.analyze-cppcheck+ cppcheck-misra histograms matchmainexactly (87 unsuppressed findings across 8 rules, identical counts) — zero regression introduced by the sweep.No new tests — pure rename + tooling sweep. Existing test suite covers the renamed constants by reference.
Areas Affected
Core/Interface/— public enum headers (SolidSyslogPrival.h,SolidSyslogDatagram.h,SolidSyslogBlockStore.h,SolidSyslogTransport.h, storage SIZE macros across the public surface).Core/Source/,Platform/*/,Tests/,Bdd/,docs/— every call site of the renamed constants..clang-tidy— single rule for enum constants.docs/NAMING.md— section rewrite + quick-ref table.S10.16 (Senders conformance, #389) resumes on top of this.
Summary by CodeRabbit
Style
Documentation
Chores