Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix build with qapt-qt6 #292

Merged
merged 1 commit into from
Jan 7, 2025
Merged

Conversation

re2zero
Copy link

@re2zero re2zero commented Jan 7, 2025

Use cmake to find the depend qapt-qt6 or qapt and link it.

Log: Fix build with qapt-qt6

Use cmake to find the depend qapt-qt6 or qapt and link it.

Log: Fix build with qapt-qt6
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. CMakeLists.txt文件中,find_package命令的REQUIRED选项确保了如果找不到指定的包,CMake会立即停止并报错。这是一个好的做法,可以避免后续的编译错误。

  2. CMakeLists.txt文件中,include_directoriesset命令用于根据QT_DESIRED_VERSION的值动态设置QApt库的版本。这种做法是合理的,但建议在设置QAPT_LIB变量之前,先检查QApt-qt6QApt是否都找到了,以避免潜在的编译错误。

  3. src/deb-installer/CMakeLists.txt文件中,target_include_directories命令的修改是合理的,因为它将DtkWidget_INCLUDE_DIRS替换为了OBJECT_BINARY_DIR。但是,如果DtkWidget_INCLUDE_DIRS是必需的,那么这个修改可能会导致编译错误。

  4. src/deepin-deb-installer-dev/CMakeLists.txt文件中,target_link_libraries命令的修改是合理的,因为它将QApt替换为了${QAPT_LIB}。这确保了链接的库是动态设置的,符合之前的逻辑。

  5. tests/CMakeLists.txt文件中,target_link_libraries命令的修改也是合理的,因为它将QApt替换为了${QAPT_LIB}。这同样确保了链接的库是动态设置的。

  6. 代码中没有明显的语法或逻辑错误。

  7. 代码中没有明显的性能问题。

  8. 代码中没有明显的安全问题。

总体来说,这些修改是合理的,但建议在设置QAPT_LIB变量之前,先检查QApt-qt6QApt是否都找到了,以避免潜在的编译错误。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lzwind, re2zero

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@re2zero
Copy link
Author

re2zero commented Jan 7, 2025

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Jan 7, 2025

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit 4e421f5 into linuxdeepin:develop/qt6.8 Jan 7, 2025
18 of 20 checks passed
@re2zero re2zero deleted the qt6 branch January 7, 2025 08:19
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.

3 participants