Skip to content

Conversation

@achamayou
Copy link
Member

@achamayou achamayou commented Nov 14, 2025

Follow-up to #7451

@achamayou achamayou changed the title [WIP] Tidy internal headers too Tidy some internal headers too Nov 26, 2025
@achamayou achamayou marked this pull request as ready for review November 26, 2025 15:48
@achamayou achamayou requested a review from a team as a code owner November 26, 2025 15:48
Copilot AI review requested due to automatic review settings November 26, 2025 15:48
Copilot finished reviewing on behalf of achamayou November 26, 2025 15:50
Copy link
Contributor

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 tidies internal headers as a follow-up to #7451, applying modern C++ best practices and code quality improvements to internal header files.

Key Changes:

  • Modernized C-style casts to static_cast and const_cast
  • Added [[nodiscard]] attributes to getter methods
  • Used default member initialization and aggregate initialization
  • Applied std::move semantics for parameter passing
  • Improved const correctness
  • Added braces to single-statement control structures
  • Replaced C-style checks with explicit comparisons
  • Extended clang-tidy header filter to cover internal headers

Reviewed changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated no comments.

Show a summary per file
File Description
.clang-tidy Extended HeaderFilterRegex to include internal header directories and disabled bugprone-branch-clone check
src/udp/msg_types.h Replaced array indexing with .data() and added aggregate initialization
src/tls/server.h Added const_cast, static_cast, and marked unused parameter
src/tls/plaintext_server.h Removed redundant virtual, added static_cast, and braces to control structures
src/tls/context.h Added const qualifiers, braces, explicit comparisons, and static_cast
src/tls/client.h Applied std::move to constructor parameter
src/tls/cert.h Changed parameters to pass-by-value with std::move
src/tls/ca.h Split assignment from condition check
src/tasks/worker.h Added inline keyword to function in header
src/tasks/task.h Added [[nodiscard]] attribute
src/tasks/ordered_tasks.h Applied std::move, [[nodiscard]], renamed parameter, and marked destructor override
src/tasks/fan_in_tasks.h Renamed parameter and marked destructor override
src/tasks/basic_task.h Applied std::move and [[nodiscard]]
src/snapshots/snapshot_manager.h Added [[nodiscard]], NOLINT comments, explicit initialization, and null checks
src/snapshots/filenames.h Added const qualifiers, split else-if, and used raw string literal
src/snapshots/fetch.h Added aggregate initialization
src/service/tables/* Added default member initialization, std::move, and explicit comparisons
src/service/network_tables.h Added [[nodiscard]] to getter methods
src/service/internal_tables_access.h Added inline, pointer markers, braces, null checks, and explicit comparisons
src/quic/quic_session.h Added enum base type, default initialization, std::move, marked destructor override, and improved control flow
src/pal/quote_generation.h Added [[maybe_unused]] and static_cast
src/consensus/ledger_enclave.h Marked unused parameters
src/consensus/aft/raft_types.h Changed destructor to default, parameter to pass-by-value, added enum size annotation
src/consensus/aft/raft.h Applied std::move, default initialization, removed redundant else clauses, added NOLINT comments
src/consensus/aft/impl/state.h Applied std::move and added [[nodiscard]]
src/common/enclave_interface_types.h Added enum base types
src/common/configuration.h Added default member initialization

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

@achamayou achamayou added the run-long-test Run Long Test job label Nov 27, 2025
@achamayou achamayou merged commit 51c92af into main Nov 27, 2025
34 checks passed
@achamayou achamayou deleted the even_tidier branch November 27, 2025 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-long-test Run Long Test job

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants