Sponsored Content
Skip to content

fix: Restore clang-tidy change to section name in config - #7091

Merged
bthomee merged 2 commits into
developfrom
bthomee/config_section
May 7, 2026
Merged

fix: Restore clang-tidy change to section name in config#7091
bthomee merged 2 commits into
developfrom
bthomee/config_section

Conversation

@bthomee

@bthomee bthomee commented May 6, 2026

Copy link
Copy Markdown
Collaborator

High Level Overview of Change

This change restores a bad clang-tidy change in a config section.

Context of Change

In #6571 the config section get call was changed from secure_gateway to secureGateway. This PR restores the name.

Copilot AI review requested due to automatic review settings May 6, 2026 23:26

@xrplf-ai-reviewer xrplf-ai-reviewer Bot 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.

No issues.

Review by Claude Opus 4.6 · Prompt: V15

@godexsoft godexsoft 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.

Thanks!

Copilot AI 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.

Pull request overview

This PR restores the expected configuration key name used by the gRPC server’s [port_grpc] config parsing, reverting a prior clang-tidy-driven rename from secure_gateway to secureGateway.

Changes:

  • Update gRPC config parsing to read secure_gateway (snake_case) instead of secureGateway (camelCase).
Comments suppressed due to low confidence (1)

src/xrpld/app/main/GRPCServer.cpp:390

  • Section::get() does an exact key lookup (no aliasing/normalization). Switching this to secure_gateway will break existing configs/tests that set secureGateway (e.g. envconfig adds (*cfg)[SECTION_PORT_GRPC].set("secureGateway", ...)). Consider accepting both keys (e.g. look up secure_gateway then fall back to secureGateway, or vice-versa with a deprecation path) to preserve backward compatibility. Also the error/log strings below still refer to the "secureGateway section" even though the config key is now secure_gateway.
        auto const optSecureGateway = section.get("secure_gateway");
        if (optSecureGateway)
        {
            try
            {
                std::stringstream ss{*optSecureGateway};
                std::string ip;
                while (std::getline(ss, ip, ','))
                {
                    boost::algorithm::trim(ip);
                    auto const addr = boost::asio::ip::make_address(ip);

                    if (addr.is_unspecified())
                    {
                        JLOG(journal_.error()) << "Can't pass unspecified IP in "
                                               << "secureGateway section of port_grpc";
                        Throw<std::runtime_error>("Unspecified IP in secureGateway section");
                    }

                    secureGatewayIPs_.emplace_back(addr);
                }
            }
            catch (std::exception const&)
            {
                JLOG(journal_.error()) << "Error parsing secure gateway IPs for grpc server";
                Throw<std::runtime_error>("Error parsing secureGateway section");
            }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bthomee bthomee added Trivial Simple change with minimal effect, or already tested. Only needs one approval. Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. and removed Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. labels May 6, 2026

@xrplf-ai-reviewer xrplf-ai-reviewer Bot 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.

No issues.

Review by Claude Opus 4.6 · Prompt: V15

@codecov

codecov Bot commented May 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.1%. Comparing base (13b72a4) to head (9be8bc4).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #7091     +/-   ##
=========================================
- Coverage     82.1%   82.1%   -0.0%     
=========================================
  Files         1010    1010             
  Lines        76023   76023             
  Branches      7372    7368      -4     
=========================================
- Hits         62403   62400      -3     
- Misses       13620   13623      +3     
Files with missing lines Coverage Δ
src/libxrpl/server/Port.cpp 74.2% <100.0%> (ø)
src/xrpld/app/main/GRPCServer.cpp 81.6% <100.0%> (ø)

... and 3 files with indirect coverage changes

Impacted file tree graph

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

@bthomee
bthomee requested a review from godexsoft May 7, 2026 00:07

@ximinez ximinez 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.

Short of re-reviewing the 60k changed lines in #6571, is there any way to confirm that this is the only string mangled this way?

@godexsoft

Copy link
Copy Markdown
Contributor

Short of re-reviewing the 60k changed lines in #6571, is there any way to confirm that this is the only string mangled this way?

Tests should have caught most things and AI did a good job finding those too during the rename PR. We will make sure nothing bad remains before these changes go into release 👍

@bthomee
bthomee added this pull request to the merge queue May 7, 2026
Merged via the queue into develop with commit 8c71ec8 May 7, 2026
3 checks passed
@bthomee
bthomee deleted the bthomee/config_section branch May 7, 2026 10:54
@mvadari mvadari added this to the 3.2.0 milestone May 20, 2026
beartec-jpg pushed a commit to beartec-jpg/FalconLedger that referenced this pull request Jun 1, 2026
Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Trivial Simple change with minimal effect, or already tested. Only needs one approval.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants