Skip to content

Conversation

@deepin-ci-robot
Copy link
Contributor

@deepin-ci-robot deepin-ci-robot commented Sep 12, 2025

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

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

Summary by Sourcery

Synchronize upstream changes from linuxdeepin/dde-session-shell by introducing a camelCase conversion helper with conditional use in key event emission, cleaning up CMake plugin entries, adding new Spanish locale translation templates, and updating reuse metadata.

New Features:

  • Add qtifyName utility to convert hyphenated names to camelCase
  • Integrate qtifyName in lockframe to emit camelCase key events under DSS_SNIPE

Enhancements:

  • Remove commented-out login-gesture plugin from CMakeLists

Documentation:

  • Add Spanish translation skeletons for es_419, es_AR, es_CL, and es_MX locales

Chores:

  • Migrate REUSE metadata from REUSE.toml to .reuse/dep5

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

Source-pull-request: linuxdeepin/dde-session-shell#43
@sourcery-ai
Copy link

sourcery-ai bot commented Sep 12, 2025

Reviewer's Guide

This PR syncs upstream changes from linuxdeepin/dde-session-shell by adding a new utility to convert dash-separated strings to camelCase, adjusting lockframe event handling to use this utility conditionally, cleaning up plugin build configuration, importing new Spanish translation templates, and updating reuse metadata.

File-Level Changes

Change Details Files
Introduce qtifyName function for camelCase conversion
  • Implement qtifyName in public_func.cpp: iterate chars, uppercase after '-'
  • Declare qtifyName with doc comment in public_func.h
src/global_util/public_func.cpp
src/global_util/public_func.h
Enhance lockframe event handling with conditional camelCase support
  • Wrap sendKeyValue call in ENABLE_DSS_SNIPE macro
  • Apply qtifyName to keyValue when DSS_SNIPE is enabled
src/dde-lock/lockframe.cpp
Clean up plugin build configuration
  • Remove commented add_subdirectory for login-gesture in plugins CMakeLists
plugins/CMakeLists.txt
Add Spanish translation template files
  • Import .ts files for es_419, es_AR, es_CL, es_MX locales under translations/
translations/dde-session-shell_es_419.ts
translations/dde-session-shell_es_AR.ts
translations/dde-session-shell_es_CL.ts
translations/dde-session-shell_es_MX.ts
Update REUSE metadata layout
  • Remove top-level REUSE.toml
  • Add .reuse/dep5 file for licensing metadata
.reuse/dep5
REUSE.toml

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

@deepin-ci-robot
Copy link
Contributor Author

deepin pr auto review

根据提供的git diff内容,我来分析一下代码变更并提出改进建议:

  1. 许可证文件变更:
  • 从REUSE.toml改为.dep5文件,这是为了更好地遵循REUSE规范
  • 建议:确保所有文件的版权声明和许可证信息完整且准确
  1. 代码变更:
  • 在lockframe.cpp中添加了ENABLE_DSS_SNIPE条件编译,用于处理键值名称转换
  • 新增了qtifyName函数用于将"some-key"转换为"someKey"格式
  • 建议:
    • 在qtifyName函数中添加输入参数验证,确保输入不为空
    • 考虑添加单元测试用例来验证转换逻辑
    • 在函数文档中说明转换规则和边界情况处理
  1. 插件配置变更:
  • 注释掉了login-gesture插件的编译
  • 建议:如果该插件不再需要,应该完全移除相关代码,而不是仅仅注释
  1. 翻译文件变更:
  • 添加了多个西班牙语翻译文件(es_419, es_AR, es_CL, es_MX)
  • 建议:
    • 确保所有翻译文本都已完成(type="unfinished"的条目需要补充翻译)
    • 统一不同地区西班牙语的术语使用
    • 检查翻译文件格式是否正确,确保每个文件都有正确的换行符
  1. 代码质量改进建议:
  • qtifyName函数实现可以优化:
    QString qtifyName(const QString &name) {
        if (name.isEmpty()) {
            return name;
        }
        
        QString result;
        result.reserve(name.size());  // 预分配空间
        bool next_cap = false;
        
        for (const QChar &c : name) {
            if (c == '-') {
                next_cap = true;
            } else if (next_cap) {
                result.append(c.toUpper());
                next_cap = false;
            } else {
                result.append(c);
            }
        }
        
        return result;
    }
  1. 安全性考虑:
  • 在处理用户输入的键值时,建议添加长度限制和特殊字符检查
  • 考虑添加日志记录,以便追踪键值转换过程
  1. 性能优化:
  • 如果qtifyName函数会被频繁调用,可以考虑使用缓存机制
  • 对于重复的转换结果,可以存储已处理的结果
  1. 维护性建议:
  • 在头文件中添加更详细的函数文档说明
  • 考虑添加相关的单元测试
  • 确保所有条件编译的代码路径都有适当的测试覆盖

这些改进建议旨在提高代码质量、性能和可维护性,同时确保代码的安全性和可靠性。

Copy link

@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 there - I've reviewed your changes - here's some feedback:

  • Wrap the new qtifyName function in an existing namespace (e.g. global_util or dss) instead of placing it in the global namespace.
  • Consider simplifying the dash-to-camelCase logic by using QString::split/join or a QRegularExpression rather than a manual character loop.
  • The ENABLE_DSS_SNIPE guard around sendKeyValue is scattered; consolidating or documenting its purpose could improve readability and maintainability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Wrap the new qtifyName function in an existing namespace (e.g. global_util or dss) instead of placing it in the global namespace.
- Consider simplifying the dash-to-camelCase logic by using QString::split/join or a QRegularExpression rather than a manual character loop.
- The ENABLE_DSS_SNIPE guard around sendKeyValue is scattered; consolidating or documenting its purpose could improve readability and maintainability.

## Individual Comments

### Comment 1
<location> `translations/dde-session-shell_es_AR.ts:1` </location>
<code_context>
+<?xml version="1.0" ?><!DOCTYPE TS><TS version="2.1" language="es_AR">
+<context>
+    <name>AuthFace</name>
</code_context>

<issue_to_address>
Consider consolidating identical Spanish translation files into a single base file and using small override files for regional differences.

Consider collapsing all of the identical `<TS … language="es_XX">` files into one base `es` file and then only shipping tiny “override” files for the locales that actually differ.  Qt will merge them at runtime if you load them in order.

1. Rename your big `es_AR.ts` into a single full `es.ts`:

   ```xml
   <?xml version="1.0" ?><!DOCTYPE TS>
   <TS version="2.1" language="es">
     <context>
       <name>AuthFace</name>
       <message>
         <source>Face ID</source>
         <translation>Face ID</translation>
       </message>
       <!-- … all 571 messages … -->
     </context>
     <!-- … other contexts … -->
   </TS>
   ```

2. Create an *override* file for Argentina only containing the strings that actually change (if there are none yet you can skip it completely):

   ```xml
   <?xml version="1.0" ?><!DOCTYPE TS>
   <TS version="2.1" language="es_AR">
     <context>
       <name>AuthFace</name>
       <message>
         <source>Face ID</source>
         <translation>ID de rostro</translation>
       </message>
     </context>
     <!-- only contexts/messages that differ -->
   </TS>
   ```

3. Load them in your `main.cpp` (or wherever you install translators) in the right order:

   ```cpp
   QTranslator base;
   base.load(":/i18n/app_es.qm");
   app.installTranslator(&base);

   QTranslator regional;
   if (QLocale::system().name() == "es_AR") {
     regional.load(":/i18n/app_es_AR.qm");
     app.installTranslator(&regional);
   }
   ```

This way you maintain **one** full Spanish file and only ship tiny diffs for each region, greatly reducing duplication.
</issue_to_address>

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.

@@ -0,0 +1,571 @@
<?xml version="1.0" ?><!DOCTYPE TS><TS version="2.1" language="es_AR">
Copy link

Choose a reason for hiding this comment

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

issue (complexity): Consider consolidating identical Spanish translation files into a single base file and using small override files for regional differences.

Consider collapsing all of the identical <TS … language="es_XX"> files into one base es file and then only shipping tiny “override” files for the locales that actually differ. Qt will merge them at runtime if you load them in order.

  1. Rename your big es_AR.ts into a single full es.ts:

    <?xml version="1.0" ?><!DOCTYPE TS>
    <TS version="2.1" language="es">
      <context>
        <name>AuthFace</name>
        <message>
          <source>Face ID</source>
          <translation>Face ID</translation>
        </message>
        <!-- … all 571 messages … -->
      </context>
      <!-- … other contexts … -->
    </TS>
  2. Create an override file for Argentina only containing the strings that actually change (if there are none yet you can skip it completely):

    <?xml version="1.0" ?><!DOCTYPE TS>
    <TS version="2.1" language="es_AR">
      <context>
        <name>AuthFace</name>
        <message>
          <source>Face ID</source>
          <translation>ID de rostro</translation>
        </message>
      </context>
      <!-- only contexts/messages that differ -->
    </TS>
  3. Load them in your main.cpp (or wherever you install translators) in the right order:

    QTranslator base;
    base.load(":/i18n/app_es.qm");
    app.installTranslator(&base);
    
    QTranslator regional;
    if (QLocale::system().name() == "es_AR") {
      regional.load(":/i18n/app_es_AR.qm");
      app.installTranslator(&regional);
    }

This way you maintain one full Spanish file and only ship tiny diffs for each region, greatly reducing duplication.

@deepin-ci-robot
Copy link
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 20f8fc4 into master Sep 18, 2025
25 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.

3 participants