feat: Issues of compile on v20 Qt5.#439
Conversation
Reviewer's GuideAdds conditional Qt5/Qt6 build support for deepin-camera on v20 by detecting Qt6 first and falling back to Qt5, adjusting modules, include paths, translation generation, and link libraries accordingly, and makes small runtime/logging fixes for camera selection and debug output. Updated class diagram for Camera and CMainWindow logging and device handlingclassDiagram
class Camera {
QString m_curDevName
void startCamera(QString devName)
}
class CMainWindow {
void onSwitchCameraSuccess(QString cameraName)
void onTimeoutLock(QString serviceName, QVariantMap key2value, QStringList argList)
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The Qt5 vs Qt6 handling in CMake duplicates quite a bit of logic (especially in QtModule and target_link_libraries); consider factoring out the common parts and only branching on the modules/targets that actually differ to keep the build configuration easier to maintain.
- In the Qt5 branch you now hardcode system include paths (/usr/include, /usr/include/libusb-1.0, etc.); if this needs to work in non-default sysroots or cross‑compile environments, it would be more robust to derive these from pkg-config variables or CMake package discovery instead of fixed paths.
- Changing m_curDevName from device.description() to devName in Camera::startCamera alters what is stored and potentially displayed as the current camera name; if UI or settings expect a human‑readable description, it may be safer to keep using the description or clearly separate the internal device ID from the user‑visible label.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The Qt5 vs Qt6 handling in CMake duplicates quite a bit of logic (especially in QtModule and target_link_libraries); consider factoring out the common parts and only branching on the modules/targets that actually differ to keep the build configuration easier to maintain.
- In the Qt5 branch you now hardcode system include paths (/usr/include, /usr/include/libusb-1.0, etc.); if this needs to work in non-default sysroots or cross‑compile environments, it would be more robust to derive these from pkg-config variables or CMake package discovery instead of fixed paths.
- Changing m_curDevName from device.description() to devName in Camera::startCamera alters what is stored and potentially displayed as the current camera name; if UI or settings expect a human‑readable description, it may be safer to keep using the description or clearly separate the internal device ID from the user‑visible label.
## Individual Comments
### Comment 1
<location> `src/src/mainwindow.cpp:715` </location>
<code_context>
tem_fontmetrics.height(), pathEditWidth);
videoPathLineEdit->setText(pi);
- qDebug() << "picPathLineEdit text:" << videoPathLineEdit->text() << Qt::endl;
+ qDebug() << "picPathLineEdit text:" << videoPathLineEdit->text();
lastVideoPath = value.toString();
option->setValue(value.toString());
</code_context>
<issue_to_address>
**suggestion:** Log label refers to `picPathLineEdit` while actually logging `videoPathLineEdit`.
The log label still says `"picPathLineEdit text:"` while outputting `videoPathLineEdit->text()`, which can be misleading when debugging. Please update the label to match `videoPathLineEdit`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
37dd930 to
17dc814
Compare
适配v20系统,完成Qt5环境的编译、打包流程。 Adapted to the v20 system, issues of compile on Qt5. v20 BUG 分支合一到v25主线 Task: https://pms.uniontech.com/task-view-383481.html
17dc814 to
e6e4148
Compare
deepin pr auto review我来帮你审查这些代码变更。主要从以下几个方面进行分析:
总体建议:
这些变更整体上是积极正面的,主要是为了支持 Qt5 和 Qt6 的兼容性,实现方式也比较合理。建议在后续维护中注意文档更新和代码注释的完善。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lichaofan2008, lzwind The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/merge |
适配v20系统,完成Qt5环境的编译、打包流程。
Adapted to the v20 system, issues of compile on Qt5.
v20 BUG 分支合一到v25主线
Task: https://pms.uniontech.com/task-view-383481.html
Summary by Sourcery
Add support for building and packaging the application with either Qt6 or Qt5, improving compatibility with the v20 system.
New Features:
Bug Fixes:
Enhancements: