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: availableSizes not work on Qt6 #255

Merged
merged 1 commit into from
Dec 12, 2024
Merged

Conversation

kegechen
Copy link
Contributor

@kegechen kegechen commented Sep 4, 2024

Qt6 QIconEngine::availableSizes will not call
virtual_hook(QIconEngine::AvailableSizesHook

deepin-ci-robot added a commit to linuxdeepin/dtk6gui that referenced this pull request Sep 4, 2024
Synchronize source files from linuxdeepin/dtkgui.

Source-pull-request: linuxdeepin/dtkgui#255
@deepin-bot
Copy link
Contributor

deepin-bot bot commented Sep 4, 2024

Doc Check bot
🟡 some documents missing!

File Line Symbol
src/util/private/dbuiltiniconengine_p.h 41 QThemeIconInfo Dtk::Gui::DBuiltinIconEngine::loadIcon

deepin-ci-robot added a commit to linuxdeepin/dtk6gui that referenced this pull request Sep 9, 2024
Synchronize source files from linuxdeepin/dtkgui.

Source-pull-request: linuxdeepin/dtkgui#255
@kegechen kegechen requested a review from 18202781743 September 9, 2024 01:32
18202781743
18202781743 previously approved these changes Sep 10, 2024
src/util/private/dbuiltiniconengine.cpp Outdated Show resolved Hide resolved
src/util/private/dciiconengine.cpp Outdated Show resolved Hide resolved
deepin-ci-robot added a commit to linuxdeepin/dtk6gui that referenced this pull request Oct 22, 2024
Synchronize source files from linuxdeepin/dtkgui.

Source-pull-request: linuxdeepin/dtkgui#255
@kegechen kegechen requested a review from zccrs October 22, 2024 02:30
@deepin-bot
Copy link
Contributor

deepin-bot bot commented Oct 31, 2024

TAG Bot

New tag: 5.7.1
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #256

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Nov 15, 2024

TAG Bot

New tag: 5.7.2
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #271

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Nov 22, 2024

TAG Bot

New tag: 5.7.3
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #273

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Dec 4, 2024

TAG Bot

New tag: 5.7.4
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #287

Qt6 QIconEngine::availableSizes will not call
virtual_hook(QIconEngine::AvailableSizesHook
deepin-ci-robot added a commit to linuxdeepin/dtk6gui that referenced this pull request Dec 10, 2024
Synchronize source files from linuxdeepin/dtkgui.

Source-pull-request: linuxdeepin/dtkgui#255
@deepin-ci-robot
Copy link
Contributor

deepin pr auto review

代码审查意见:

  1. 代码重复

    • DBuiltinIconEngineDDciIconEngine类中,availableSizes函数的实现非常相似。建议提取公共代码到一个单独的函数中,以减少重复并提高代码的可维护性。
  2. 默认参数

    • availableSizes函数中,modestate参数有默认值。虽然这在某些情况下可能是有用的,但最好在文档中明确说明这些参数的默认值,以便其他开发者理解。
  3. 性能优化

    • DBuiltinIconEngineDDciIconEngineavailableSizes函数中,使用了reserve方法来预分配QList的大小。这是一个好的做法,因为它可以减少在列表增长时需要重新分配内存的次数。但是,如果m_info.entries.size()availableSizes.size()的值非常大,那么预分配可能并不总是有效。在这种情况下,应该考虑其他优化策略,比如使用std::vector代替QList
  4. 错误处理

    • DBuiltinIconEngineDDciIconEngineavailableSizes函数中,没有对m_info.entriesm_dciIcon.availableSizes返回的值进行空检查。如果这些值可能为空,应该添加相应的空检查以避免潜在的空指针异常。
  5. 代码注释

    • DBuiltinIconEngineDDciIconEngineavailableSizes函数中,应该添加注释来解释函数的目的、参数的含义以及返回值的含义。这有助于其他开发者理解代码的意图。
  6. 版本控制

    • DBuiltinIconEngineDDciIconEngineavailableSizes函数中,使用了#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)宏来区分不同版本的Qt。这是一个好的做法,因为它可以确保代码在不同版本的Qt上都能正确编译。但是,应该确保在文档中说明这些函数的版本兼容性,以便其他开发者了解如何使用这些函数。

综上所述,代码中存在一些可以改进的地方,包括减少重复代码、优化性能、添加错误处理和注释,以及确保版本控制的一致性。

@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, kegechen, zccrs

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

@18202781743 18202781743 merged commit 4ff3a64 into linuxdeepin:master Dec 12, 2024
22 of 24 checks passed
18202781743 pushed a commit to linuxdeepin/dtk6gui that referenced this pull request Dec 12, 2024
Synchronize source files from linuxdeepin/dtkgui.

Source-pull-request: linuxdeepin/dtkgui#255
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.

4 participants