Skip to content

Conversation

@Neverlord
Copy link
Member

I meant to look into #365 again to see if newer clang-tidy versions maybe fixed the issue. Turns out our clang-tidy CI check doesn't do anything anymore since we've moved our sources from src to libbroker. 🤦‍♂️

These are some changes clang-tidy-19 has suggested. Unfortunately, it did suggest changes that break the code, so we can't just update CI and start running this check with 19 again. Suppressing every suggestion that broke the code would add a lot of noise to the code. So before bringing the check back in a future PR, I'll see if I can maybe restructure the code slightly and otherwise disable noisy checks.

Copy link

Copilot AI left a comment

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 applies code improvements suggested by clang-tidy-19 to enhance performance and code quality. The changes include passing parameters by const reference where appropriate, adding noexcept specifiers to swap functions, using designated initializers, adding reserve() calls before loop insertions, and modernizing C++ code style.

Key Changes:

  • Parameters changed to const references to avoid unnecessary copies
  • Added noexcept to swap functions for better move semantics
  • Added reserve() calls to optimize vector allocations
  • Replaced nested std::min calls with initializer list syntax
  • Updated namespace declarations to modern C++ style
  • Added override keyword to virtual destructor

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
libbroker/broker/store.test.cc Changed temporary objects to named variables for default construction test
libbroker/broker/radix_tree.test.cc Pass parameters by const reference, use CHECK_EQUAL with unsigned literals
libbroker/broker/peering.test.cc Pass parameters by const reference, add reserve() for vector optimization
libbroker/broker/internal/channel.test.cc Pass parameters by const reference to avoid copies
libbroker/broker/format/bin.test.cc Pass parameters by const reference, use designated initializers
libbroker/broker/detail/radix_tree.hh Add noexcept to swap functions, modernize namespace syntax, simplify std::min calls
libbroker/broker/builder.test.cc Pass parameter by const reference
libbroker/broker/backend.test.cc Add override keyword and reserve() call
libbroker/broker/alm/routing_table.test.cc Pass parameter by const reference
libbroker/broker/alm/multipath.test.cc Pass parameter by const reference, add reserve() call

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

@Neverlord Neverlord force-pushed the topic/neverlord/clang-tidy branch from 15abddd to 8f89dc8 Compare December 13, 2025 16:03
@Neverlord Neverlord marked this pull request as ready for review December 13, 2025 16:11
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