Skip to content

Conversation

@copi143
Copy link
Collaborator

@copi143 copi143 commented Dec 11, 2025

No description provided.

Copilot AI review requested due to automatic review settings December 11, 2025 10:48
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 reorganizes header file imports to minimize dependencies and optimizes compilation parameters for faster debug builds. The changes replace monolithic includes (e.g., opencv2/opencv.hpp) with specific module includes and remove unused headers throughout the codebase.

Key changes:

  • Split opencv2/opencv.hpp into specific module includes (imgcodecs.hpp, imgproc.hpp, videoio.hpp, core.hpp) across multiple files
  • Added debug-specific compilation flags (-Og) and precompiled headers for spdlog to speed up debug builds
  • Removed unused header includes across the codebase (QCoreApplication, QFileInfo, convert.h, vector, QTextEdit, etc.)

Reviewed changes

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

Show a summary per file
File Description
CMakeLists.txt Added compile commands export, corrected version script comment path, configured debug optimization flags, added precompiled headers for debug builds, and removed VERSION_CPP from glob to prevent conflicts
src/mqtt/mqtt_client.cpp Added blank line after header include for consistency
src/mqtt/MQTTMessageWidget.cpp Repositioned blank line after header include
src/main.cpp Removed unused convert.h include
src/logging.h Removed unused QCoreApplication include
src/convert.h Removed unused vector include, split opencv2/opencv.hpp into specific imgcodecs and imgproc includes
src/components/message_dialog.cpp Removed QVBoxLayout include (incorrectly - still used in code), added blank line
src/components/UiConfig.cpp Removed unused QFileInfo include, added blank line
src/about_dialog.cpp Removed unused QApplication include, added blank line
src/FrameWidget.cpp Added blank lines for better code organization
src/CameraWidget.h Removed unused QTextEdit include, replaced opencv2/opencv.hpp with specific videoio.hpp
src/CameraWidget.cpp Removed multiple unused Qt includes, added specific opencv2 includes (imgcodecs, imgproc)
src/BarcodeWidget.h Reordered includes to group by type (project headers, then system headers)
src/BarcodeWidget.cpp Replaced opencv2/opencv.hpp with specific includes (core, imgcodecs, imgproc), added blank lines

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

if(NOT MSVC)
set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -Og")
set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -Og")
set(CMAKE_EXE_LINKER_FLAGS_DEBUG "${CMAKE_EXE_LINKER_FLAGS_DEBUG} -O0 -Wl,-O0")
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linker flags include -O0 -Wl,-O0 which disables optimization for linking. While this may speed up link time, it's unusual to optimize the C/CXX compilation with -Og while disabling linker optimization completely. Consider using -Wl,-O1 instead of -Wl,-O0 to maintain some link-time optimization benefits while still improving debug build times.

Suggested change
set(CMAKE_EXE_LINKER_FLAGS_DEBUG "${CMAKE_EXE_LINKER_FLAGS_DEBUG} -O0 -Wl,-O0")
set(CMAKE_EXE_LINKER_FLAGS_DEBUG "${CMAKE_EXE_LINKER_FLAGS_DEBUG} -O0 -Wl,-O1")

Copilot uses AI. Check for mistakes.
@copi143
Copy link
Collaborator Author

copi143 commented Dec 11, 2025

难绷微软服务爆炸导致ci失败

@Mq-b Mq-b force-pushed the master branch 2 times, most recently from 112f843 to 8c765ab Compare December 12, 2025 02:17
@Mq-b
Copy link
Owner

Mq-b commented Dec 13, 2025

或许你应该说明一下这是根据什么定义的include顺序,也让后面的人一直按照它。

@copi143
Copy link
Collaborator Author

copi143 commented Dec 13, 2025

include顺序由clang-format配置文件定义,我建议也在项目头文件和依赖头文件间添加一个空行。

@Mq-b
Copy link
Owner

Mq-b commented Dec 13, 2025

在 docs 写个文档介绍一下一些基本的吧

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