feat: support run command in runtime or base environment#1669
Conversation
There was a problem hiding this comment.
Code Review
This pull request generalizes the container layer and item retrieval logic by transitioning from app-specific layers to target layers (which can fall back to runtime or base layers). The review feedback focuses on improving error handling across these changes: specifically, the reviewer recommends preserving the underlying error context by passing .error() to the error-handling macros (LINGLONG_ERR and LINGLONG_ERRV) when getCachedTargetItem() or getTargetLayer() fails, and printing the error instead of silently returning -1 in Cli::run.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
Signed-off-by: reddevillg <reddevillg@gmail.com>
deepin pr auto review你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff。本次代码变更的主要目的是将运行上下文中对“应用层”的硬依赖,泛化为对“目标层”的依赖,使得在没有应用层的情况下,可以回退使用运行时层或基础层。这是一个很好的功能增强,提升了程序的健壮性和灵活性。 以下是我对本次变更在语法逻辑、代码质量、代码性能和代码安全方面的详细审查意见及改进建议: 1. 语法与逻辑
2. 代码质量
3. 代码性能
4. 代码安全
总结与修改示例本次修改逻辑清晰,很好地支持了非 App 场景的运行。主要需要修正的是错误宏的传参问题,以及建议优化嵌套三元表达式。 针对 // 修改前
if (!targetItem) {
this->printer.printErr(LINGLONG_ERRV("failed to get cached target item", targetItem));
return -1;
}
// 修改后
if (!targetItem) {
this->printer.printErr(LINGLONG_ERRV("failed to get cached target item"));
return -1;
}针对 // 修改前
.package = !info->app.empty()
? info->app
: (info->runtime && !info->runtime->empty() ? *info->runtime : info->base),
// 修改后 (假设 C++17,使用 std::string_view 或 const std::string& 避免拷贝)
const auto& pkg = !info->app.empty()
? info->app
: (info->runtime && !info->runtime->empty() ? *info->runtime : info->base);
myContainers.emplace_back(api::types::v1::CliContainer{
.id = std::move(info->containerID),
.package = pkg, // 如果类型是 string,这里可能需要 std::string(pkg)
.pid = container->pid,
});希望这些审查意见对你有所帮助!如果有任何疑问,欢迎随时讨论。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dengbo11, reddevillg The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
No description provided.