Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fixed the issue that MPV library could not be found #503

Merged
merged 1 commit into from
Aug 14, 2024
Merged

fix: Fixed the issue that MPV library could not be found #503

merged 1 commit into from
Aug 14, 2024

Conversation

pengfeixx
Copy link
Contributor

@pengfeixx pengfeixx commented Aug 14, 2024

Fixed the issue that MPV library could not be found

Bug: https://pms.uniontech.com/bug-view-265013.html
Log: Fixed the issue that MPV library could not be found

@myk1343
Copy link
Contributor

myk1343 commented Aug 14, 2024

[是否满足兼容性要求] Y
[是否满足commit提交规范] Y
[是否满足编码规范] Y
[Review结论] Pass
[Fail原因] N/A

Fixed the issue that MPV library could not be found

Bug: https://pms.uniontech.com/bug-view-265013.html
Log: Fixed the issue that MPV library could not be found
@deepin-ci-robot
Copy link

deepin pr auto review

关键摘要:

  • 代码简化:移除了冗余的ifelse语句,通过!list.isEmpty()直接返回结果,提高了代码的简洁性。

是否建议立即修改:

  • 否,提交的代码更改看起来是一个合理的优化,没有发现需要立即修改的问题。但是,建议进行以下几点额外的审查:
    • 确认QLibraryInfo::location(QLibraryInfo::LibrariesPath)的路径设置是否正确,以确保dir.setPath(path)能够正确设置路径。
    • 检查dir.entryList的调用是否有可能抛出异常,例如在路径不存在或没有权限访问时。
    • 确认QStringList() << (QString("libmpv.so") + "*")的构造是否正确处理了libmpv.so的扩展名,以及是否考虑了其他可能的动态链接库文件名。
    • 如果isMpvExists函数被频繁调用,考虑是否有必要缓存结果以提高性能。
    • 确保代码遵循了项目的编码标准和最佳实践,例如代码格式和注释。

Copy link
Contributor

@rb-union rb-union left a comment

Choose a reason for hiding this comment

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

[是否满足兼容性要求] Y
[是否满足commit提交规范] Y
[是否满足编码规范] Y
[Review结论] Pass
[Fail原因] N/A

Copy link
Contributor

@myk1343 myk1343 left a comment

Choose a reason for hiding this comment

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

+1

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: myk1343, pengfeixx, rb-union

The full list of commands accepted by this bot can be found here.

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

@pengfeixx pengfeixx merged commit 84a9aa5 into linuxdeepin:release/eagle Aug 14, 2024
15 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.

4 participants