Skip to content

refactor: Part of Remove Qt dependencies from utils#1573

Merged
ComixHe merged 1 commit into
OpenAtom-Linyaps:masterfrom
reddevillg:remove_qt_3
Jan 28, 2026
Merged

refactor: Part of Remove Qt dependencies from utils#1573
ComixHe merged 1 commit into
OpenAtom-Linyaps:masterfrom
reddevillg:remove_qt_3

Conversation

@reddevillg

Copy link
Copy Markdown
Collaborator

Move Qt/DBus related utilities from utils to common.

Note: This commit changes the namespace linglong::utils::dbus to linglong::common::dbus and linglong::utils::serialize (partially) to linglong::common::serialize.

Move Qt/DBus related utilities from utils to common.

Note: This commit changes the namespace `linglong::utils::dbus` to
`linglong::common::dbus` and `linglong::utils::serialize` (partially)
to `linglong::common::serialize`.

Signed-off-by: reddevillg <reddevillg@gmail.com>
@deepin-ci-robot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: reddevillg

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

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @reddevillg, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request systematically refactors core utility components by relocating Qt and D-Bus specific functionalities from the linglong::utils module to a newly established linglong::common module. This strategic move aims to reduce the utils module's dependency on Qt, promoting a cleaner separation of concerns and making utils more generic and reusable. The changes involve updating namespaces, file paths, and usage patterns across the application, ensuring that Qt-dependent logic resides in common while utils retains its general-purpose nature.

Highlights

  • Relocation of D-Bus Utilities: Components related to D-Bus, such as PropertiesForwarder and register functions, have been moved from linglong::utils::dbus to linglong::common::dbus. This includes renaming files and updating namespaces.
  • Relocation of GKeyFile Wrapper: The GKeyFileWrapper utility, used for handling GKeyFile configurations, has been moved from linglong::utils to linglong::common, along with its associated test files.
  • Partial Relocation of Serialization Logic: Qt-specific serialization functions (e.g., toQVariantMap, fromQVariantMap, QJsonObjectfromVariantMap) have been extracted from linglong::utils::serialize and moved to linglong::common::serialize. The linglong::utils::serialize module now focuses on non-Qt specific JSON serialization.
  • Improved Logging and Error Handling: Qt-specific logging calls (qFatal, qCritical, qCDebug) have been replaced with generic LogE and LogD macros, and std::abort() for critical errors, further decoupling core utilities from Qt's logging system.
  • Centralized Formatter: A new linglong::common::formatter has been introduced to provide fmt library specializations for QString, QStringList, and GError, ensuring consistent formatting across the codebase.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@deepin-ci-robot

Copy link
Copy Markdown
Collaborator

deepin pr auto review

代码审查报告

总体概述

这次变更主要是将 linglong::utils::dbuslinglong::utils::serialize 相关的代码从 utils 库移动到 linglong 库的 common 命名空间下。这是一个代码重构操作,目的是更好地组织代码结构。

语法和逻辑分析

1. 命名空间变更

变更将以下内容从 linglong::utils 命名空间移动到 linglong::common 命名空间:

  • dbus 相关功能
  • serialize 相关功能
  • GKeyFileWrapper
  • formatter 相关功能

这个变更在语法上是正确的,逻辑上也是合理的,因为这些功能确实更偏向于通用(common)功能而非特定工具(utils)功能。

2. 头文件引用变更

变更更新了所有引用这些头文件的地方,从 linglong/utils/xxx 改为 linglong/common/xxx。这些变更都是一致的,没有遗漏。

代码质量评估

优点

  1. 代码组织更清晰:将通用功能移到 common 命名空间下,使代码结构更合理
  2. 一致性:所有相关文件和引用都进行了统一更新
  3. 测试更新:测试文件也相应地进行了移动和更新

可能的改进点

  1. 命名空间一致性:虽然 common 命名空间更合适,但需要确保整个项目中所有类似功能都使用相同的命名空间策略
  2. 文档更新:如果有相关文档,需要同步更新

代码性能分析

这次变更主要是代码重构,不涉及性能相关的修改。代码的性能不会因为命名空间的变更而受到影响。

代码安全评估

  1. 错误处理

    • properties_forwarder.cpp 中,将 qFatal 替换为 LogEstd::abort() 是一个改进,因为 qFatal 会直接终止程序,而新的实现提供了日志记录。
    if (parent == nullptr) {
  •  qFatal("parent is nullptr");
    
  •  LogE("parent is nullptr");
    
  •  std::abort();
    
    }
    
    
  1. 类型安全

    • serialize/json.h 中的模板函数保持了类型安全,没有引入新的安全隐患。
  2. 资源管理

    • GKeyFileWrapper 使用了 std::unique_ptr 来管理资源,确保资源正确释放。

具体改进建议

  1. 错误处理改进
    properties_forwarder.cpp 中,可以考虑使用更具体的错误类型而不是直接调用 std::abort(),例如:

    if (parent == nullptr) {
        LogE("parent is nullptr");
        return LINGLONG_ERR("parent cannot be nullptr");
    }
  2. 日志记录改进
    register.cpp 中,可以考虑添加更详细的日志信息,例如注册失败时的原因:

    if (conn.lastError().isValid()) {
        LogE("Failed to register dbus name {}: {}", serviceName.toStdString(), 
             conn.lastError().message().toStdString());
        return LINGLONG_ERR(conn.lastError().name() + ": " + conn.lastError().message());
    }
  3. 类型转换安全性
    formatter.cpp 中,QStringstd::string 的转换可能会失败(例如包含非ASCII字符),可以考虑添加错误处理:

    auto fmt::formatter<QString>::format(const QString &qstr, fmt::format_context &ctx) const
      -> fmt::format_context::iterator
    {
        // 使用 QString::toUtf8() 而不是 toStdString() 以更好地处理非ASCII字符
        return formatter<std::string_view>::format(qstr.toUtf8().toStdString(), ctx);
    }

总结

这次代码重构变更整体上是积极的,它提高了代码组织的合理性,没有引入明显的性能或安全问题。主要的改进点在于错误处理和日志记录方面,可以考虑进一步优化这些方面以提高代码的健壮性和可维护性。

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request is a significant and well-executed refactoring that moves Qt and D-Bus related utilities from the utils library to a new common library, effectively reducing dependencies in utils. The changes are consistent with the stated goal and improve the project's architecture.

I've identified a few areas where error messages in the new D-Bus registration functions could be more descriptive to aid in debugging. These are minor points in an otherwise solid refactoring effort.

return LINGLONG_ERR(conn.lastError().name() + ": " + conn.lastError().message());
}

return LINGLONG_ERR("unknown");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The error message "unknown" is not very descriptive. It would be better to provide a more specific message indicating why unregisterDBusService might have failed, for example, if the service was not registered by this connection or the name is invalid.

Suggested change
return LINGLONG_ERR("unknown");
return LINGLONG_ERR(QString("Failed to unregister D-Bus service %1. It might not have been registered by this connection or the name is invalid.").arg(serviceName));

@codecov

codecov Bot commented Jan 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 4.62963% with 103 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...bs/linglong/src/linglong/common/serialize/json.cpp 0.00% 24 Missing ⚠️
...g/src/linglong/package_manager/package_manager.cpp 0.00% 17 Missing ⚠️
libs/linglong/src/linglong/cli/cli.cpp 0.00% 15 Missing ⚠️
libs/linglong/src/linglong/common/serialize/json.h 0.00% 12 Missing ⚠️
libs/linglong/src/linglong/repo/ostree_repo.cpp 0.00% 12 Missing ⚠️
libs/linglong/src/linglong/common/formatter.cpp 0.00% 8 Missing ⚠️
.../src/linglong/common/dbus/properties_forwarder.cpp 0.00% 7 Missing ⚠️
libs/linglong/src/linglong/common/dbus/register.h 0.00% 4 Missing ⚠️
...long/src/linglong/package_manager/package_task.cpp 0.00% 2 Missing ⚠️
libs/linglong/src/linglong/cli/cli.h 0.00% 1 Missing ⚠️
... and 1 more
Files with missing lines Coverage Δ
apps/ll-package-manager/src/main.cpp 0.00% <ø> (ø)
...s/linglong/src/linglong/builder/source_fetcher.cpp 71.15% <ø> (ø)
...bs/linglong/src/linglong/common/gkeyfile_wrapper.h 44.92% <100.00%> (ø)
libs/linglong/src/linglong/package/layer_file.cpp 29.09% <ø> (ø)
...nglong/src/linglong/package_manager/package_task.h 21.73% <ø> (ø)
libs/utils/src/linglong/utils/serialize/json.h 76.92% <ø> (+41.20%) ⬆️
libs/linglong/src/linglong/cli/cli.h 8.33% <0.00%> (ø)
libs/utils/src/linglong/utils/log/formatter.cpp 50.00% <0.00%> (+20.00%) ⬆️
...long/src/linglong/package_manager/package_task.cpp 48.54% <0.00%> (ø)
libs/linglong/src/linglong/common/dbus/register.h 0.00% <0.00%> (ø)
... and 7 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

const auto *mo = parent->metaObject();
if (mo == nullptr) {
qFatal("couldn't find metaObject of parent.");
LogE("couldn't find metaObject of parent.");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

要不加个LogF?

@ComixHe ComixHe merged commit 22fc444 into OpenAtom-Linyaps:master Jan 28, 2026
15 of 16 checks passed
@reddevillg reddevillg deleted the remove_qt_3 branch January 28, 2026 09:01
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