Skip to content

Conversation

@yixinshark
Copy link
Contributor

1.Divide plugins into three groups(selfMaintenance,application,other) 2.single process support to start multiple plugins

Log: as title
Bug: https://pms.uniontech.com/bug-view-283901.html

@yixinshark yixinshark force-pushed the chore-trayLoaderLoadMultPlugins branch 2 times, most recently from b491876 to 5b413b0 Compare December 4, 2024 02:31
@yixinshark yixinshark requested a review from tsic404 December 4, 2024 02:36
@yixinshark yixinshark force-pushed the chore-trayLoaderLoadMultPlugins branch from 5b413b0 to 54c2e6b Compare December 4, 2024 02:37
"name[zh_CN]": "自维护托盘插件",
"description": "self maintenance plugins",
"description[zh_CN]": "自维护托盘插件",
"permissions": "readwrite",
Copy link
Contributor

Choose a reason for hiding this comment

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

这个是不是应该只读🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

"serial": 0,
"flags": [],
"name": "application tray plugins",
"name[zh_CN]": "应用托盘插件",
Copy link
Contributor

Choose a reason for hiding this comment

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

应该是子项目托盘插件,不是应用托盘吧

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

"name[zh_CN]": "应用托盘插件",
"description": "application tray plugins",
"description[zh_CN]": "应用托盘插件",
"permissions": "readwrite",
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

if (dirs.isEmpty())
dirs << pluginDirs;

QString selfMaintenancePluginPaths;
Copy link
Contributor

Choose a reason for hiding this comment

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

可以声明为一个list, 用join(";")去拼接成一个字符串。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

"permissions": "readwrite",
"visibility": "private"
},
"selfMaintenanceTrayPlugins": {
Copy link
Contributor

Choose a reason for hiding this comment

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

我们自己维护的插件,能不能改插件类型,在meta中加个字段,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

想,但目前不是很允许。

@yixinshark yixinshark force-pushed the chore-trayLoaderLoadMultPlugins branch 2 times, most recently from ded970b to 0ea3291 Compare December 4, 2024 08:39
const QString otherPlugins = "otherPlugins";

auto dConfig = Dtk::Core::DConfig::create("org.deepin.dde.shell", "org.deepin.ds.dock.tray", QString());
QStringList selfMaintenanceTrayPlugins = dConfig->value("selfMaintenanceTrayPlugins").toStringList();
Copy link
Contributor

Choose a reason for hiding this comment

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

这下面的key与上面的变量是不是重复的?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

可以是一致,done

for (auto &pluginDir : dirs) {
QDir dir(pluginDir);
if (!dir.exists()) {
qWarning() << "The plugin directory does not exist:" << pluginDir;
Copy link
Contributor

Choose a reason for hiding this comment

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

category?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个地方是遍历插件目录

return validExePath;
}

QMap<QString, QString> LoadTrayPlugins::getPluginGroup() const
Copy link
Contributor

Choose a reason for hiding this comment

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

要不抽出一个函数吧,传入满足的条件,返回结果,例如QStringList xxx(const QStringList &plugins) const;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

抽一个获取所有插件的函数,和给插件分组的函数。

@yixinshark yixinshark force-pushed the chore-trayLoaderLoadMultPlugins branch 3 times, most recently from 254a5af to cf9a3b2 Compare December 5, 2024 06:12
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: tsic404, yixinshark

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

@yixinshark yixinshark force-pushed the chore-trayLoaderLoadMultPlugins branch from cf9a3b2 to f46b80a Compare December 6, 2024 06:57
1.Divide plugins into three groups(selfMaintenance,application,other)
2.single process support to start multiple plugins

Log: as title
Bug: https://pms.uniontech.com/bug-view-283901.html
@yixinshark yixinshark force-pushed the chore-trayLoaderLoadMultPlugins branch from f46b80a to 0910903 Compare December 6, 2024 06:59
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. 依赖项版本更新

    • debian/control文件中,将dde-tray-loader的版本从=更新为>,这可能会导致与旧版本的不兼容。需要确认新版本是否向后兼容,并确保所有相关组件都已更新。
  2. 配置文件新增字段

    • org.deepin.ds.dock.tray.json文件中新增了三个字段:selfMaintenanceTrayPluginssubprojectTrayPluginscrashProneTrayPlugins。这些字段的添加是否与现有代码逻辑兼容,以及是否有相应的单元测试来验证这些新字段的正确性。
  3. 代码重构

    • loadtrayplugins.cpp文件中,对加载托盘插件的逻辑进行了重构,将一些重复的代码提取到了新的函数中,如startProcessloaderPathallPluginPaths。这提高了代码的可读性和可维护性。
  4. 错误处理

    • loadtrayplugins.cpp中,当找不到有效的加载器可执行文件路径时,使用了qWarning输出警告信息。建议考虑是否需要将此信息记录到日志文件中,以便于后续的调试和维护。
  5. 资源管理

    • loadtrayplugins.cpp中,为每个托盘插件创建了一个新的QProcess实例,并在handleProcessFinished槽函数中管理这些进程。需要确保在进程结束后正确地释放资源,避免内存泄漏。
  6. 配置文件读取

    • loadtrayplugins.cpp中,通过Dtk::Core::DConfig读取配置文件中的插件列表。需要确保配置文件路径的正确性,并且配置文件中的插件列表格式正确。
  7. 代码注释

    • loadtrayplugins.cpp中,新增的函数和字段缺少必要的注释说明。建议添加注释,以便其他开发者理解代码的意图和用途。
  8. 安全性

    • loadtrayplugins.cpp中,加载托盘插件时使用了QProcess,需要确保插件路径的合法性,避免执行恶意代码。可以考虑使用安全的方法来验证插件路径。

总体来说,代码重构和错误处理是这次提交的主要改进点,但还需要注意依赖项的兼容性、配置文件的处理、资源管理和安全性等方面的问题。

@yixinshark
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Dec 6, 2024

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit d8963ca into linuxdeepin:master Dec 6, 2024
7 of 10 checks passed
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