Skip to content

Conversation

@deepin-ci-robot
Copy link
Contributor

Synchronize source files from linuxdeepin/dtkcore.

Source-pull-request: linuxdeepin/dtkcore#527

@deepin-ci-robot
Copy link
Contributor Author

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: deepin-ci-robot

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

Details 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

Synchronize source files from linuxdeepin/dtkcore.

Source-pull-request: linuxdeepin/dtkcore#527
@deepin-ci-robot
Copy link
Contributor Author

deepin pr auto review

我来对这个diff进行代码审查:

  1. 版本控制逻辑:
  • 代码使用QT_VERSION_CHECK来控制不同Qt版本的兼容性,这是很好的做法。
  • 但版本控制逻辑有些复杂,建议添加注释说明不同版本间的API变化原因。
  1. 代码结构:
  • 在头文件中,版本判断的顺序从QT_VERSION >= QT_VERSION_CHECK(6, 8, 0)改为了先判断QT_VERSION >= QT_VERSION_CHECK(6, 9, 0),这样的改变可能暗示了6.9版本有新的API变化。
  • 建议在代码中添加注释说明6.9版本的具体变化。
  1. 性能考虑:
  • beginEntryList的实现看起来没有性能问题,都是简单的委托给DDciFileEngineIterator。
  1. 安全性:
  • 没有发现明显的安全问题。

改进建议:

  1. 添加版本变更说明:
// Qt 6.9 introduces new API for directory listing
#if QT_VERSION >= QT_VERSION_CHECK(6, 9, 0)
    IteratorUniquePtr beginEntryList(const QString &path, QDirListing::IteratorFlags filters, const QStringList &filterNames) override;
    IteratorUniquePtr endEntryList() override;
// Qt 6.8 introduces QDirListing::IteratorFlags
#elif QT_VERSION >= QT_VERSION_CHECK(6, 8, 0)
    IteratorUniquePtr beginEntryList(const QString &path, QDirListing::IteratorFlags filters, const QStringList &filterNames) override;
    IteratorUniquePtr beginEntryList(const QString &path, QDir::Filters filters, const QStringList &filterNames) override;
    IteratorUniquePtr endEntryList() override;
  1. 在cpp文件中也添加相应的注释:
#if QT_VERSION < QT_VERSION_CHECK(6, 9, 0)
// Legacy implementation for Qt versions before 6.9
#if QT_VERSION >= QT_VERSION_CHECK(6, 8, 0)
    QAbstractFileEngine::IteratorUniquePtr DDciFileEngine::beginEntryList(const QString &path, QDir::Filters filters, const QStringList &filterNames)
#else
    DDciFileEngine::Iterator *DDciFileEngine::beginEntryList(QDir::Filters filters, const QStringList &filterNames)
#endif
{
    Q_UNUSED(path);
    return new DDciFileEngineIterator(filters, filterNames);
}
#endif
  1. 考虑添加版本检查的辅助宏来简化代码:
#define QT_VERSION_AT_LEAST(major, minor, patch) (QT_VERSION >= QT_VERSION_CHECK(major, minor, patch))

#if QT_VERSION_AT_LEAST(6, 9, 0)
    // ...
#elif QT_VERSION_AT_LEAST(6, 8, 0)
    // ...
#endif
  1. 建议添加单元测试来验证不同Qt版本下的行为一致性。

这些改进将使代码更易维护,并且更容易理解版本间的API变化原因。

@18202781743 18202781743 merged commit 52314ed into master Dec 17, 2025
13 of 15 checks passed
@18202781743 18202781743 deleted the sync-pr-527-nosync branch December 17, 2025 06:33
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