Skip to content

Fix linter warnigns found using the QT agentic skills - #768

Open
j-rivero wants to merge 4 commits into
mainfrom
lint/qt-cpp-cleanup
Open

Fix linter warnigns found using the QT agentic skills#768
j-rivero wants to merge 4 commits into
mainfrom
lint/qt-cpp-cleanup

Conversation

@j-rivero

Copy link
Copy Markdown
Contributor

🎉 New feature

Summary

Found that the Qt is supplying an agentic development kit that can review cpp and QML, see the introduction in https://www.qt.io/blog/introducing-qt-agentic-development-skills.

I put my agent to work on this code and the first pass found many interesting points to improve. In this first PR I address the complains done by the linter: https://github.com/TheQtCompanyRnD/agent-skills/tree/main/skills/qt-cpp-review#what-it-does.

One commit per issue detected with a meaningful commit message. I reviewed the commits and they made sense to me.

Test it

I've build a colcon workspace with gz-gui and gz-sim and the test suite run fine on my 24.04 Ubuntu. I was also able to launch gz-sim and play with the GUI and plot just fine.

Checklist

  • Signed all commits for DCO
  • Added a screen capture or video to the PR description that demonstrates the feature
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • Updated Bazel files (if adding new files). Created an issue otherwise.
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers
  • Was GenAI used to generate this PR? If so, make sure to add "Generated-by" to your commits. (See this policy for more info.)

Generated-by: Claude Opus 4.7

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by and Generated-by messages.

Backports: If this is a backport, please use Rebase and Merge instead.

j-rivero added 4 commits May 22, 2026 12:03
Convert 22 direct-list-init sites (Type var{value};) to copy-init
(Type var = value;) across MainWindow.hh Q_PROPERTY shadow members,
Application.cc local config-load variables, and Dialog.cc.

Copy-init form avoids surprises from narrowing conversions on scalars
with explicit-constructor types while staying terse. No behavioral
change.

Generated-By: Claude Opus 4.7
Signed-off-by: Jose Luis Rivero <jrivero@honurobotics.com>
The loop body in Application::PluginByName only reads the iterated
shared_ptr; the non-const reference could detach a Qt COW container
if pluginsAdded were ever switched to one. Tighten to const auto&.

Generated-By: Claude Opus 4.7
Signed-off-by: Jose Luis Rivero <jrivero@honurobotics.com>
Prefer .size() for consistency with the rest of the std container
API. Two trivial sites; no behavior change.

Generated-By: Claude Opus 4.7
Signed-off-by: Jose Luis Rivero <jrivero@honurobotics.com>
…MO-1)

Change the internal Implementation::timeout member from int to
std::chrono::milliseconds to remove unit ambiguity. QTimer::setInterval
binds to its chrono overload (Qt 5.8+), so InitTimer() does not need
explicit conversion. UpdateTime() now reads .count() before scaling
to seconds. Public Timeout() API and Q_PROPERTY are unchanged.

Generated-By: Claude Opus 4.7
Signed-off-by: Jose Luis Rivero <jrivero@honurobotics.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Inbox

Development

Successfully merging this pull request may close these issues.

2 participants