Skip to content

clang-tidy: resolve readability-const-return-type in virtual - #770

Merged
greenc-FNAL merged 1 commit into
mainfrom
maintenance/fix-virtual-const-return-type
Jul 28, 2026
Merged

clang-tidy: resolve readability-const-return-type in virtual#770
greenc-FNAL merged 1 commit into
mainfrom
maintenance/fix-virtual-const-return-type

Conversation

@greenc-FNAL

@greenc-FNAL greenc-FNAL commented Jul 27, 2026

Copy link
Copy Markdown
Contributor
  • Code quality: Resolved readability-const-return-type for the virtual mode() accessor.
  • API consistency: Updated IStorage_File, Storage_File, and its implementation to return char instead of top-level const char.
  • Behavior: No functional behavior changed.

@greenc-FNAL
greenc-FNAL requested a review from knoepfel July 27, 2026 21:35
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4b6db7e1-1dcb-4eb3-964c-4212579d7863

📥 Commits

Reviewing files that changed from the base of the PR and between 0be155e and e41f458.

📒 Files selected for processing (3)
  • form/storage/istorage.hpp
  • form/storage/storage_file.cpp
  • form/storage/storage_file.hpp
📜 Recent review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: build (gcc, none)
  • GitHub Check: coverage
  • GitHub Check: clang-tidy-check
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{cpp,cc,cxx,h,hpp}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{cpp,cc,cxx,h,hpp}: Use clang-format tool for all C++ code formatting (VS Code auto-formats on save); configuration defined in .clang-format with 100-character line limit and 2-space indentation
Follow clang-tidy recommendations defined in .clang-tidy

Files:

  • form/storage/istorage.hpp
  • form/storage/storage_file.hpp
  • form/storage/storage_file.cpp
**/*.{hpp,cpp}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{hpp,cpp}: Use .hpp for header files, .cpp for implementation, and *_test.cpp for test files in C++
Enforce 100-character line limit and 2-space indentation in C++ code via .clang-format
Use QualifierAlignment: Right (east-const) style: int const x not const int x in C++
Use PointerAlignment: Left in C++ (pointer * attached to type, not variable name)
All C++ identifiers must use lower_case naming: namespaces, classes, structs, enums, functions, variables, parameters, members, and constants
Exception to C++ naming: template parameters use CamelCase
Exception to C++ naming: macros use UPPER_CASE
Private, protected, and constant members in C++ must have a trailing underscore (_), no trailing underscore on anything else
Use enum class preferred over plain enum in C++
Use std::shared_ptr for shared ownership, std::unique_ptr for exclusive ownership, raw pointers for non-owning references only in C++
Use functors with agent-noun pattern: ModelEvaluator evaluate_model(...) in C++
Apply .clang-tidy checks for bugprone, cert, clang-analyzer, concurrency, cppcoreguidelines, misc, modernize, performance, portability, and readability as defined in the .clang-tidy configuration file
Use phlex:: namespace for core code, phlex::experimental:: for experimental features in C++

Files:

  • form/storage/istorage.hpp
  • form/storage/storage_file.hpp
  • form/storage/storage_file.cpp
**/*.hpp

📄 CodeRabbit inference engine (AGENTS.md)

Avoid boolean parameters in C++ interfaces; prefer enumerations instead

Files:

  • form/storage/istorage.hpp
  • form/storage/storage_file.hpp
🔇 Additional comments (3)
form/storage/istorage.hpp (1)

56-56: LGTM!

form/storage/storage_file.hpp (1)

17-17: LGTM!

form/storage/storage_file.cpp (1)

11-11: LGTM!


📝 Walkthrough

Walkthrough

The storage file mode() accessor now returns char instead of char const consistently across its interface, override declaration, and implementation.

Changes

Storage mode return type

Layer / File(s) Summary
Align mode signatures
form/storage/istorage.hpp, form/storage/storage_file.hpp, form/storage/storage_file.cpp
The mode() interface, override, and implementation now return char while preserving the returned member value.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Suggested reviewers: knoepfel

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing a clang-tidy const-return-type warning on a virtual accessor.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch maintenance/fix-virtual-const-return-type

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@            Coverage Diff             @@
##             main     #770      +/-   ##
==========================================
- Coverage   83.82%   83.81%   -0.02%     
==========================================
  Files         172      172              
  Lines        7210     7210              
  Branches      848      848              
==========================================
- Hits         6044     6043       -1     
- Misses        884      885       +1     
  Partials      282      282              
Flag Coverage Δ
scripts 80.09% <ø> (ø)
unittests 85.68% <100.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
form/storage/istorage.hpp 100.00% <ø> (ø)
form/storage/storage_file.cpp 100.00% <100.00%> (ø)
form/storage/storage_file.hpp 100.00% <ø> (ø)

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0be155e...e41f458. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@greenc-FNAL
greenc-FNAL merged commit 8315603 into main Jul 28, 2026
37 checks passed
@greenc-FNAL
greenc-FNAL deleted the maintenance/fix-virtual-const-return-type branch July 28, 2026 13:18
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.

2 participants