Skip to content

Conversation

@asterwyx
Copy link
Contributor

Compositor may need frame_done signal to manage opened dmabuf attributes.

@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

@deepin-bot
Copy link

deepin-bot bot commented Nov 21, 2024

TAG Bot

New tag: 0.4.3
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #12

@asterwyx asterwyx changed the title WIP: add frame_done request feat: add frame_done request Nov 25, 2024
@asterwyx asterwyx marked this pull request as ready for review November 25, 2024 05:39
@asterwyx asterwyx requested a review from justforlxz November 25, 2024 05:39
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. XML文件格式:XML文件格式正确,没有发现明显的格式错误。

  2. 新增frame_done请求

    • 新增的frame_done请求描述清晰,但建议在<description>标签中添加一个<example>标签,以提供使用该请求的示例代码,帮助开发者理解如何正确使用该请求。
  3. 修改cancel事件的描述

    • 修改后的描述更加清晰,使用了“Notifies”来明确表示这是一个通知事件,而不是描述事件本身。
  4. 修改枚举值other的描述

    • other枚举值的描述从“other failure”修改为“User cancel this context from compositor”,这可能会引起误解,因为other通常表示其他未分类的错误。建议保留原有的“other failure”描述,或者为user_cancel添加一个新的枚举值来明确表示用户取消的情况。
  5. 代码注释和文档

    • 虽然代码本身没有问题,但建议在XML文件中添加更多的注释和文档,特别是在复杂的事件和请求中,以帮助开发者更好地理解和使用这些功能。
  6. 性能和安全性

    • 由于这是一个XML配置文件,性能和安全性问题相对较少。但是,确保XML解析库的使用是安全的,避免XML注入攻击。

总体来说,代码的修改是合理的,但需要注意一些细节,以提高代码的可读性和可维护性。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: asterwyx, justforlxz

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

@justforlxz justforlxz merged commit f2feb7d into linuxdeepin:master Nov 25, 2024
1 of 5 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