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 bug #93

Merged
merged 2 commits into from
Dec 6, 2024
Merged

fix bug #93

merged 2 commits into from
Dec 6, 2024

Conversation

Johnson-zs
Copy link
Contributor

fix os_version error

- Change default COMPILE_ON_V23 value to true
- Modify version detection to check for V20 instead of V23
- Set COMPILE_ON_V23 to false when V20 is detected
- Update compilation messages for better clarity

Log:

Bug: https://pms.uniontech.com/bug-view-287059.html
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. 逻辑错误

    • common.pri文件中,COMPILE_ON_V23的初始值被设置为true,然后根据OS_VERSION_FILE的内容再次设置为false。如果OS_VERSION_FILE中包含MajorVersion为20,则COMPILE_ON_V23被设置为false。但是,如果OS_VERSION_FILE中包含MajorVersion为23或更高版本,COMPILE_ON_V23应该保持为true。当前的逻辑会导致COMPILE_ON_V23始终为false,这可能不是预期的行为。
  2. 代码可读性

    • common.pri文件中,注释和代码的混合使用使得代码的可读性降低。建议将注释和代码分开,以提高代码的可读性。
  3. 错误处理

    • common.pri文件中,当OS_VERSION_FILE不存在时,使用error函数抛出错误。这是一个好的做法,但是错误信息应该更加详细,以便于调试和修复问题。
  4. 版本控制

    • debian/changelog文件中,提交信息应该更加详细,说明修复了什么问题,以及如何修复的。当前的提交信息只提到了“修复了一个bug”,但没有说明具体的bug是什么,以及修复的方法。
  5. 拼写错误

    • debian/changelog文件中,提交信息中的“os_version”拼写错误,应该是“os-version”。

综合以上意见,建议对代码进行如下修改:

  1. 修复common.pri文件中的逻辑错误,确保COMPILE_ON_V23的设置符合预期。
  2. 优化代码的可读性,将注释和代码分开。
  3. common.pri文件中,增加详细的错误信息,以便于调试和修复问题。
  4. debian/changelog文件中,增加详细的提交信息,说明修复了什么问题,以及修复的方法。
  5. 修正拼写错误,将“os_version”改为“os-version”。

Copy link

github-actions bot commented Dec 5, 2024

TAG Bot

TAG: 5.0.21
EXISTED: no
DISTRIBUTION: unstable

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Clauszy, Johnson-zs

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

@Johnson-zs Johnson-zs merged commit 78ecf29 into linuxdeepin:master Dec 6, 2024
18 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