Actionable comments posted: 1

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.clang-tidy:
- Around line 2-5: Update the .clang-tidy configuration so that Checks expands
beyond only bugprone-string-constructor to include the targeted groups (e.g.,
modernize-*, cppcoreguidelines-*, readability-*, performance-*) and avoid making
every warning an error immediately; modify the Checks field to include those
groups and change WarningsAsErrors from "*" to either an explicit list of rule
names or remove the wildcard (leave empty) while you run clang-tidy to generate
a baseline/fix list, then reintroduce WarningsAsErrors once the baseline
violations are fixed; refer to the existing Keys "Checks" and "WarningsAsErrors"
in the file to implement these changes.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9a73baac-3251-4324-a91a-ead949838aec

Reviewing files that changed from the base of the PR and between e480ac2 and 9bff6f1.

• .clang-tidy
• .github/workflows/ci.yml

Pull request overview

Adds a repository-wide clang-tidy configuration and introduces a CI job to run clang-tidy as a quality gate, aligning with issue #553’s goal of enforcing more modern C++ usage and reducing technical debt.

Changes:

• Added a root .clang-tidy configuration file.
• Added a new tidy job in GitHub Actions CI to run xmake check clang.tidy using that config.
• Minor CI YAML formatting changes (line wrapping / spacing).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

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

Actionable comments posted: 1

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/plugin/physics/src/system/ConstraintHelpers.cpp`:
- Around line 94-99: The exception handler for storing a ConstraintInternal must
not let cleanup throw: ensure ctx.physicsSystem.RemoveConstraint(joltConstraint)
is called inside a protective no-throw block (e.g., wrap it in a try-catch) or
use a non-throwing cleanup helper so that joltConstraint->Release() always runs
even if RemoveConstraint throws; reference the same pattern used in
DestroyConstraint() and apply it to the catch handling around RemoveConstraint
and joltConstraint->Release().

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 88c50c10-8de2-4166-b7ce-b8747b3a4076

Reviewing files that changed from the base of the PR and between 672628c and 1d6040f.

• src/plugin/physics/src/system/ConstraintHelpers.cpp

src/utils/log/src/Logger.hpp (2)

40-58: ⚡ Quick win

Optional: collapse the if/else if dispatch in Log::Log using spdlog::log.

Now that ToSpdlogLevel maps reliably via static_cast, the entire manual dispatch chain can be replaced with a single spdlog::log call. This removes ~12 lines of boilerplate and eliminates the latent risk of silently missing a newly added Log::Level value (e.g., the current chain falls through to Trace as the default rather than forwarding trace explicitly).

♻️ Proposed simplification

 template <typename T> inline void Log(Level level, const T &msg) noexcept
 {
-    using enum Log::Level;
-
-    if (level == info)
-        Log::Info(msg);
-    else if (level == warning)
-        Log::Warning(msg);
-    else if (level == error)
-        Log::Error(msg);
-    else if (level == critical)
-        Log::Critical(msg);
-    else if (level == debug)
-        Log::Debug(msg);
-    else if (level == off)
-        return;
-    else
-        Log::Trace(msg);
+    spdlog::log(ToSpdlogLevel(level), msg);
 };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/utils/log/src/Logger.hpp` around lines 40 - 58, Replace the manual
if/else dispatch in the template function Log (the Log::Log overload) with a
single spdlog::log call using the existing ToSpdlogLevel conversion: first
early-return on Level::off if you want to suppress logging, then call
spdlog::log(ToSpdlogLevel(level), msg). Remove the long if/else chain and the
"using enum Log::Level;" line; keep the template signature and rely on
ToSpdlogLevel(Level) to map levels so new enum values won't silently fall
through.

---

13-13: Pin Log::Level's underlying type to int to match spdlog::level::level_enum explicitly.

The static_cast on line 25 is valid because both Log::Level and spdlog::level::level_enum use int as their underlying type. However, spdlog::level::level_enum declares this explicitly (: int), while Log::Level relies on implicit default. Adding : int to Log::Level makes the coupling explicit and consistent, preventing any accidental incompatibility if the enum representation ever changes.

♻️ Proposed change

-enum class Level {
+enum class Level : int {
     trace    = spdlog::level::level_enum::trace,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/utils/log/src/Logger.hpp` at line 13, The enum Log::Level should
explicitly pin its underlying type to int to match spdlog::level::level_enum;
update the declaration of enum class Level to use ": int" (e.g., enum class
Level : int) so the static_cast to spdlog::level::level_enum (used elsewhere in
Logger.hpp) is guaranteed safe and stable if defaults change.

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/utils/log/src/Logger.hpp`:
- Around line 40-58: Replace the manual if/else dispatch in the template
function Log (the Log::Log overload) with a single spdlog::log call using the
existing ToSpdlogLevel conversion: first early-return on Level::off if you want
to suppress logging, then call spdlog::log(ToSpdlogLevel(level), msg). Remove
the long if/else chain and the "using enum Log::Level;" line; keep the template
signature and rely on ToSpdlogLevel(Level) to map levels so new enum values
won't silently fall through.
- Line 13: The enum Log::Level should explicitly pin its underlying type to int
to match spdlog::level::level_enum; update the declaration of enum class Level
to use ": int" (e.g., enum class Level : int) so the static_cast to
spdlog::level::level_enum (used elsewhere in Logger.hpp) is guaranteed safe and
stable if defaults change.

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f854ab1b-93a1-4ab7-9f5b-36b895eb7d53

Reviewing files that changed from the base of the PR and between 1d6040f and 95f7bc1.

• .clang-tidy
• src/utils/log/src/Logger.hpp

• .clang-tidy