Skip to content

clang-tidy: resolve modernize-concat-nested-namespaces - #804

Merged
marcpaterno merged 1 commit into
Framework-R-D:mainfrom
knoepfel:modernize-concat-nested-namespaces
Aug 17, 2026
Merged

clang-tidy: resolve modernize-concat-nested-namespaces#804
marcpaterno merged 1 commit into
Framework-R-D:mainfrom
knoepfel:modernize-concat-nested-namespaces

Conversation

@knoepfel

@knoepfel knoepfel commented Aug 5, 2026

Copy link
Copy Markdown
Member

Resolve modernize-concat-nested-namespaces clang-tidy warnings.

  • Code quality

    • Replaced nested namespace syntax in form/form/technology.hpp with namespace form::technology.
    • Preserved all technology constants, helper functions, values, and behavior.
    • Resolved modernize-concat-nested-namespaces clang-tidy warnings.
  • Documentation

    • Marked modernize-concat-nested-namespaces as complete.
    • Added the associated PR #804 reference to docs/dev/clang-tidy-fixes-2026-04.md.

knoepfel added a commit to knoepfel/phlex that referenced this pull request Aug 5, 2026
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 16dd9f19-6be7-4f93-bb6a-3646f0163d9b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR replaces nested namespace syntax in form/form/technology.hpp with namespace form::technology. It also marks the related clang-tidy checklist item complete and adds the PR reference.

Changes

Namespace modernization

Layer / File(s) Summary
Modernize namespace syntax and tracking
form/form/technology.hpp, docs/dev/clang-tidy-fixes-2026-04.md
The technology declarations use C++ nested namespace syntax without changing values or behavior. The checklist records the completed modernize-concat-nested-namespaces fix and PR reference.

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

Possibly related PRs

Suggested reviewers: beojan

🚥 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 clearly identifies the clang-tidy warning resolved by the code and documentation changes.
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 unit tests (beta)
  • Create PR with unit tests

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 Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
form/form/technology.hpp 66.66% 1 Missing ⚠️
@@            Coverage Diff             @@
##             main     #804      +/-   ##
==========================================
+ Coverage   83.89%   84.03%   +0.13%     
==========================================
  Files         171      171              
  Lines        7234     7234              
  Branches      850      850              
==========================================
+ Hits         6069     6079      +10     
+ Misses        881      874       -7     
+ Partials      284      281       -3     
Flag Coverage Δ
scripts 80.09% <ø> (ø)
unittests 86.00% <66.66%> (+0.20%) ⬆️

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

Files with missing lines Coverage Δ
form/form/technology.hpp 66.66% <66.66%> (ø)

... 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 240a461...1f75054. Read the comment docs.

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

pcanal
pcanal previously approved these changes Aug 11, 2026

@pcanal pcanal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@knoepfel
knoepfel force-pushed the modernize-concat-nested-namespaces branch from 1f75054 to 32a72ed Compare August 17, 2026 18:16
@marcpaterno
marcpaterno merged commit 03891a9 into Framework-R-D:main Aug 17, 2026
37 checks passed
@pcanal

pcanal commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Something went odd on this PR. I only see the documentation part left as part of this PR but the commit log mentions this same PR ( #804 ) where it probably meant to refer to #816

@knoepfel

Copy link
Copy Markdown
Member Author

Something went odd on this PR. I only see the documentation part left as part of this PR but the commit log mentions this same PR ( #804 ) where it probably meant to refer to #816

Sorry to be confusing, @pcanal. When the PR was originally created, it included the fixes that concatenate nested namespaces in FORM. Then PR #816 was merged, which addressed the problem. The log on this PR was not updated, though. The commit history, however, should be clear.

@knoepfel
knoepfel deleted the modernize-concat-nested-namespaces branch August 17, 2026 19:34
@pcanal

pcanal commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Nothing (more) we can do about it but the commit log of 03891a9 is mentioning #804. However this conversation will disambiguate the issue for future readers :)

@knoepfel

Copy link
Copy Markdown
Member Author

Nothing (more) we can do about it but the commit log of 03891a9 is mentioning #804. However this conversation will disambiguate the issue for future readers :)

Argh...you're right. I thought the commit message was fixed. Oops. Sigh.

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.

3 participants