refactor: align identifier naming with house style + re-enable the gate#124
Open
facontidavide wants to merge 1 commit into
Open
refactor: align identifier naming with house style + re-enable the gate#124facontidavide wants to merge 1 commit into
facontidavide wants to merge 1 commit into
Conversation
Correct the readability-identifier-naming policy to match the documented house
style and re-enable the check as a WarningsAsErrors gate (it was "temporarily
disabled during rename migration"). The SDK already follows the policy closely;
this is a config alignment plus a small internal-only cleanup.
.clang-tidy amendments:
- LocalConstant/LocalVariable = lower_case: const/constexpr *locals* stay
lower_case; the k-prefix Constant policy applies to file-scope/global
constants only (without this, const locals falsely flag en masse).
- NamespaceIgnoredRegexp '^(PJ|Ui)$': the flat `PJ` namespace (house style) and
uic-generated `Ui`.
- C-ABI / canonical-schema / STL exemptions (the plugin binary + wire contract):
* Struct/Enum/EnumConstant/Function/GlobalConstant `PJ_`/`pj_` extern "C"
C-ABI identifiers, plus the camelBack vtable member `fetchMessageData`.
* PublicMember single-capital `D K R P` (ROS/OpenCV CameraInfo/DepthImage
intrinsics -- a wire-format field contract).
* Method has_value/value_or/error_or (std::expected drop-in interface) and
load_config/save_config/ui_content/widget_data/trampoline_* (plugin-author
and C-ABI-bridge methods every compiled plugin links by name).
* TypeAlias `json` (nlohmann alias).
Recasing any of these breaks wire compat, STL-shaped generic code, or the
plugin ABI -- they are grandfathered (revisit the snake_case plugin virtuals
in a future coordinated 1.0.0).
- StructIgnoredRegexp `overloaded` (std::visit idiom).
- ParameterIgnoredRegexp '.*_': a trailing underscore on a parameter is the
idiomatic shadow-avoidance form (`PayloadView(Span bytes_) : bytes(bytes_)`);
stripping it reintroduces a -Wshadow error, so trailing-underscore params are
exempt.
Internal-only renames (no public API/ABI change -- local variables, file-local
helpers, test/example code):
flatten_impl/count_leaf_fields_impl -> camelBack; test helpers (ColorEq,
ColorNear, schema_release, array_release, stream_release, make_robot_pose)
-> camelBack; local vars (requiredString/withPath/writeField/...) -> lower_case;
named constants (ns_per_second, default_a_/default_b_) -> k-prefix.
Versioning: config + invisible internal renames only; no installed public API,
ABI struct, or wire-format change, and abi/baseline.abi is untouched -> no
version bump warranted.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
What
Corrects the
readability-identifier-namingpolicy to match documented house style and re-enables the check as aWarningsAsErrorsgate (it was "temporarily disabled during rename migration"). The SDK already follows the policy closely — this is a config alignment plus a small internal-only cleanup. No public API / ABI / wire-format change.This is the SDK-side companion to the PJ4 app rename (PJ4#215); the two repos were intentionally split so the SDK's plugin contract is handled with ABI awareness here.
.clang-tidyamendmentsLocalConstant/LocalVariable = lower_caselower_case; thek-prefix Constant rule applies to file-scope/global constants onlyNamespaceIgnoredRegexp '^(PJ|Ui)$'PJnamespace (house style) + uic-generatedUiPJ_/pj_exemptions (Struct/Enum/EnumConstant/Function/GlobalConstant) +fetchMessageDataPublicMember [A-Z]D K R P— a wire-format field contractMethod has_value|value_or|error_or+load_config|save_config|ui_content|widget_data|trampoline_*std::expecteddrop-in interface + plugin-author / C-ABI-bridge methods linked by nameTypeAlias json,Struct overloaded,Parameter '.*_'std::visitidiom; trailing-_param shadow-avoidance idiomThe grandfathered snake_case plugin virtuals (
save_config,trampoline_*, …) are left as-is — renaming them is a coordinated 1.0.0 break (every plugin recompiles); flagged for a future major.Internal-only renames (no API/ABI impact)
flatten_impl/count_leaf_fields_impl→ camelBack · test helpers (ColorEq,schema_release,make_robot_pose, …) → camelBack · local vars →lower_case· named constants (ns_per_second,default_a_) →k-prefix.Versioning
Config + invisible internal renames only — no installed public API, ABI struct, or wire-format change;
abi/baseline.abiuntouched. No version bump warranted.Verification
pj_base/pj_pluginstests)🤖 Generated with Claude Code