sync: from linuxdeepin/dde-session-shell#494
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds an additional condition to the shutdown-inhibit acceptance logic in WarningContent so that shutdown/restart actions are only accepted when they specifically request shutdown/restart and the model’s password check passes, instead of being allowed for broader power actions. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new compound condition in
doAcceptShutdownInhibitinverts the previous logic forRequireShutdown/RequireRestart(from!=to==and addinggsCheckpwd()), which is a behavioral change rather than a straight sync; double-check whether the intent is to restrict this branch only to those two actions instead of excluding them as before. - Consider extracting the
m_powerActionchecks indoAcceptShutdownInhibitinto a small helper (e.g.,isRequireShutdownOrRestart()) or at least adding parentheses for clarity, as the current nested boolean expression is harder to read and easy to misinterpret.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new compound condition in `doAcceptShutdownInhibit` inverts the previous logic for `RequireShutdown`/`RequireRestart` (from `!=` to `==` and adding `gsCheckpwd()`), which is a behavioral change rather than a straight sync; double-check whether the intent is to restrict this branch only to those two actions instead of excluding them as before.
- Consider extracting the `m_powerAction` checks in `doAcceptShutdownInhibit` into a small helper (e.g., `isRequireShutdownOrRestart()`) or at least adding parentheses for clarity, as the current nested boolean expression is harder to read and easy to misinterpret.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
4b2369a to
60e5835
Compare
Synchronize source files from linuxdeepin/dde-session-shell. Source-pull-request: linuxdeepin/dde-session-shell#61
60e5835 to
fe24a1f
Compare
deepin pr auto review代码审查报告1. 语法逻辑审查问题:逻辑运算符优先级与括号使用不当 在修改后的代码中: && ((m_powerAction == SessionBaseModel::RequireShutdown || m_powerAction == SessionBaseModel::RequireRestart) && m_model->gsCheckpwd())
建议: 2. 代码质量审查问题:可读性
建议: bool isPowerAction = (m_powerAction == SessionBaseModel::RequireShutdown ||
m_powerAction == SessionBaseModel::RequireRestart);
if (m_model->currentModeState() != SessionBaseModel::ModeStatus::PowerMode &&
m_powerAction != SessionBaseModel::RequireUpdateShutdown &&
m_powerAction != SessionBaseModel::RequireUpdateRestart &&
(!isPowerAction || m_model->gsCheckpwd())) {
// ...
}3. 代码性能审查问题:函数调用次数
建议: bool isPasswordChecked = m_model->gsCheckpwd();
if (/* ... */ && (!isPowerAction || isPasswordChecked)) {
// ...
}4. 代码安全审查问题:潜在的安全漏洞
建议:
5. 其他建议
总结
如果需要进一步的帮助,请提供更多上下文或业务逻辑说明! |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: deepin-ci-robot, yixinshark 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 |
Synchronize source files from linuxdeepin/dde-session-shell.
Source-pull-request: linuxdeepin/dde-session-shell#61
Summary by Sourcery
Bug Fixes: