Skip to content

Conversation

@wineee
Copy link
Member

@wineee wineee commented Mar 14, 2025

Log: refork kwin form uos, don't rename

Summary by Sourcery

Chores:

  • Rename deepin-kwin to kwin.

Log: refork kwin form uos, don't rename
@deepin-ci-robot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

log: rename deepin-kwin to kwin
@github-actions
Copy link

TAG Bot

TAG: 1.99.12
EXISTED: no
DISTRIBUTION: unstable

@wineee
Copy link
Member Author

wineee commented Mar 24, 2025

/integr-topic kwin-qt6

@deepin-ci-robot
Copy link

Alreadly latest topic integration with deepin-community/Repository-Integration#2693

@wineee
Copy link
Member Author

wineee commented Mar 24, 2025

/integr-topic kwin-qt6

@deepin-ci-robot
Copy link

Alreadly latest topic integration with deepin-community/Repository-Integration#2693

@wineee
Copy link
Member Author

wineee commented Mar 24, 2025

/topic kwin-qt6

@deepin-ci-robot
Copy link

Add topic: kwin-qt6 successed.

@wineee wineee marked this pull request as ready for review March 25, 2025 06:50
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. 文件路径硬编码

    • othersmanager.cpp文件中,路径/etc/xdg/kglobalshortcutsrc$HOME/.config/kglobalshortcutsrc是硬编码的,建议使用QStandardPaths来获取这些路径,以保持代码的可移植性。
  2. 错误处理

    • othersmanager.cpp文件中,当QFile(config).exists()返回false时,没有进行任何处理。建议添加错误处理逻辑,例如记录日志或抛出异常。
  3. 资源管理

    • othersmanager.cpp文件中,QFile configFile(config);创建了一个文件对象,但没有看到相应的configFile.close();调用。建议使用QScopedFileHandle来自动管理文件资源。
  4. 字符串拼接

    • othersmanager.cpp文件中,QStandardPaths::standardLocations(QStandardPaths::ConfigLocation).first() + "/kwinrc"使用了字符串拼接,建议使用QDirfilePath方法来拼接路径,以避免路径分隔符的问题。
  5. 代码注释

    • othersmanager.cpp文件中,建议添加更多的注释来解释代码的目的和逻辑,特别是对于复杂的条件判断和文件操作。
  6. 服务文件更新

    • dde-session@x11.service文件中,ExecStart命令从/usr/bin/deepin-kwin_x11更改为/usr/bin/kwin_x11,但没有更新相应的文档或配置说明。建议在更新服务文件时,同时更新相关的文档和配置说明,确保一致性。
  7. 日志记录

    • othersmanager.cpp文件中,当QFile(configFile).open(QIODevice::ReadOnly)失败时,使用了qWarning()来记录错误。建议使用qCritical()来记录严重错误,以便于问题追踪。
  8. 代码风格

    • othersmanager.cpp文件中,建议保持代码风格的一致性,例如缩进和空格的使用。

以上是针对代码审查提出的改进意见,希望能够帮助到开发者。

@sourcery-ai
Copy link

sourcery-ai bot commented Mar 25, 2025

Reviewer's Guide by Sourcery

This pull request renames deepin-kwin to kwin in the codebase. The changes involve updating the executable name and the kwinrc config file path in othersmanager.cpp.

No diagrams generated as the changes look simple and do not need a visual representation.

File-Level Changes

Change Details Files
The pull request renames deepin-kwin to kwin in the codebase.
  • Renamed deepin-kwin_x11 to kwin_x11.
  • Updated the kwinrc config file path from deepin-kwinrc to kwinrc.
src/dde-session/othersmanager.cpp

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!
  • Generate a plan of action for an issue: Comment @sourcery-ai plan on
    an issue to generate a plan of action for it.

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

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

Overall Comments:

  • Consider using a variable for the window manager executable name to avoid repetition.
  • It might be good to update any related documentation or comments that mention the old name.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

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.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wineee, zccrs

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

@wineee wineee merged commit 469ea64 into linuxdeepin:master Apr 1, 2025
17 of 18 checks passed
@github-project-automation github-project-automation bot moved this to Done in kwin-qt6 Apr 1, 2025
@wineee wineee deleted the kwin branch April 1, 2025 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants