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: crash when saving avatar #1960

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

FeiWang1119
Copy link

change Type::Cartoon to Type::Dimensional

pms: BUG-295675

change Type::Cartoon to Type::Dimensional

pms: BUG-295675
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. 变量命名

    • m_currentAvatarLsv 变量名中的 Lsv 可能是 ListView 的缩写,但这个缩写不够明确,建议使用更清晰的命名,如 m_currentAvatarListView
  2. 类型转换

    • AvatarListFrame 构造函数中,Type::Dimensional 被用作 m_avatarViewMap.value() 的键。确保 Type 枚举中确实存在 Dimensional 值,并且与之前的 Cartoon 值相对应。
  3. 逻辑判断

    • AvatarListDialog 构造函数中,if (index.row() == 4)avatarSelectWidget->setCurrentIndex(4) 的逻辑可能需要重新考虑。如果 Custom 常量被定义为 4,那么这个逻辑是正确的。但是,如果 Custom 的值发生变化,这个逻辑可能会出错。建议使用 Custom 常量来避免硬编码的魔法数字。
  4. 代码风格

    • AvatarListDialog 构造函数中,if (!path.isEmpty() && path != m_curUser->currentAvatar()) 语句后面的代码块应该缩进,以保持代码的可读性。
  5. 注释

    • AvatarListDialog 构造函数中,// 成功设置头像后关闭窗口 注释应该放在 accept(); 语句之前,以保持注释与代码的同步。
  6. 代码重复

    • AvatarListDialog 构造函数中,accept(); 被重复调用两次。建议将重复的代码合并,以减少代码重复。
  7. 连接信号和槽

    • AvatarListDialog 构造函数中,connect 语句的 lambda 表达式中的 this 指针可能不需要,因为 lambda 表达式已经捕获了 this 指针。如果 this 指针确实需要,应该明确指出。

综上所述,代码在变量命名、类型转换、逻辑判断、代码风格、注释、代码重复和连接信号和槽等方面存在一些改进空间。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: FeiWang1119, kegechen

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

@FeiWang1119 FeiWang1119 merged commit 4236fab into linuxdeepin:release/6.0.78 Dec 18, 2024
15 of 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