Skip to content

sync: from linuxdeepin/dde-session-shell#500

Merged
yixinshark merged 1 commit into
masterfrom
sync-pr-65-nosync
May 7, 2026
Merged

sync: from linuxdeepin/dde-session-shell#500
yixinshark merged 1 commit into
masterfrom
sync-pr-65-nosync

Conversation

@deepin-ci-robot
Copy link
Copy Markdown
Contributor

@deepin-ci-robot deepin-ci-robot commented May 7, 2026

Synchronize source files from linuxdeepin/dde-session-shell.

Source-pull-request: linuxdeepin/dde-session-shell#65

Summary by Sourcery

Bug Fixes:

  • Set the notification appName for the lock screen to the dde-lock identifier instead of a translated label.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented May 7, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Updates the lock screen notification to use a stable application identifier and synchronizes the corresponding desktop entry metadata from upstream.

Sequence diagram for updated lock screen notification app identifier

sequenceDiagram
    participant LockContent
    participant DBusSessionBus
    participant NotificationsService as org_freedesktop_Notifications

    LockContent->>DBusSessionBus: method Notify(appName: dde-lock, replacesId: 0, appIcon: empty, summary: empty, body: ...)
    DBusSessionBus->>NotificationsService: Notify(appName: dde-lock, replacesId: 0, appIcon: empty, summary: empty, body: ...)
    NotificationsService-->>DBusSessionBus: notificationId
    DBusSessionBus-->>LockContent: notificationId
Loading

File-Level Changes

Change Details Files
Use a fixed application name identifier when sending desktop notifications from the lock screen and sync the related desktop entry.
  • Replace the translated "Lock Screen" display string with the literal application identifier "dde-lock" in the org.freedesktop.Notifications Notify call so notifications are associated with the dde-lock app rather than a localized name.
  • Synchronize the dde-lock.desktop file from upstream (likely aligning Name, Exec, and related metadata with the new application identifier and other upstream changes).
src/session-widgets/lockcontent.cpp
files/snipe/dde-lock.desktop

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • Switching the notification app name from a translatable string to the fixed "dde-lock" removes localization of the app name in notifications; confirm this is intentional and, if not, consider keeping a user-facing summary separate from the fixed app identifier.
  • For the constant string "dde-lock", consider using a more lightweight literal type (e.g., QLatin1String or QStringLiteral) instead of QString("...") to avoid an unnecessary allocation.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Switching the notification app name from a translatable string to the fixed "dde-lock" removes localization of the app name in notifications; confirm this is intentional and, if not, consider keeping a user-facing summary separate from the fixed app identifier.
- For the constant string "dde-lock", consider using a more lightweight literal type (e.g., QLatin1String or QStringLiteral) instead of QString("...") to avoid an unnecessary allocation.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Synchronize source files from linuxdeepin/dde-session-shell.

Source-pull-request: linuxdeepin/dde-session-shell#65
@deepin-ci-robot
Copy link
Copy Markdown
Contributor Author

deepin pr auto review

这段代码的修改主要涉及两处:桌面配置文件的本地化更新和C++源文件中的版权年份更新及通知参数修改。以下是我的审查意见:

1. 语法逻辑

代码逻辑正确

  • 桌面文件(.desktop)的本地化字符串添加符合 freedesktop.org 规范
  • C++ 代码的版权年份更新和参数修改在语法上都是正确的

2. 代码质量

优点

  • 多语言支持全面,包含了英语、简体中文、繁体中文(台湾/香港)、维吾尔语和藏语
  • 使用了 SPDX 标识符进行版权声明

改进建议

  1. 版权年份范围:将版权年份改为"2022 - 2026"可能不太合适,建议使用"2022 - present"或仅更新到当前年份(如2023或2024),除非确定会维护到2026年。

  2. 代码注释

.arg(QString("dde-lock"))     // appName

建议保持这个注释,因为它清晰地说明了参数的用途,有助于代码可读性。

3. 代码性能

无性能问题

  • 这两处修改都不会对性能产生明显影响
  • 桌面文件的本地化字符串在系统启动时加载一次
  • 通知参数的修改只是字符串常量的替换

4. 代码安全

潜在问题
将通知的应用名称从本地化字符串 tr("Lock Screen") 改为硬编码的 "dde-lock"

优点

  • 避免了潜在的本地化问题
  • 确保通知系统中的应用名称一致
  • 减少了翻译负担

建议

  • 如果 dde-lock 是应用程序的固定标识符,这种修改是合理的
  • 但如果未来需要在不同地区显示不同的应用名称,可能需要考虑其他方案

5. 其他建议

  1. 桌面文件

    • 确保所有添加的本地化字符串都经过了专业翻译人员的审核
    • 考虑添加更多语言支持(如日语、韩语等)以扩大用户基础
  2. 版本控制

    • 建议在提交信息中说明这些修改的原因
    • 特别是关于通知参数的修改,应该说明为什么要从本地化字符串改为固定标识符

总结

这段代码的修改整体上是合理的,主要改进了国际化支持和通知系统的一致性。唯一需要注意的是版权年份的设置,以及确保"dde-lock"作为固定标识符是符合项目需求的。

@deepin-ci-robot
Copy link
Copy Markdown
Contributor Author

[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.

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 merged commit 8f77ea9 into master May 7, 2026
26 of 29 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.

2 participants