-
Notifications
You must be signed in to change notification settings - Fork 40
refactor(enum): rename special computer type enums to consistent nami… #596
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
Conversation
…ng convention - Rename kCustomType to kSpecialType8 - Add new kSpecialType9 enum value - Update all references throughout codebase to use new enum names - Add support for kSpecialType9 in CPU frequency and monitor display logic log: rename special computer type enums to consistent naming convention task: https://pms.uniontech.com/task-view-385813.html Change-Id: Id3da3d2b9ca530992ecddff69beb0da7a8fffcb6
deepin pr auto review这段代码主要进行了对特殊计算机类型枚举值的重构,将原本语义不明确的命名(如PGUW、KLVV等)统一替换为具有通用性的命名规范(kSpecialType1-9),并扩展了对新类型(kSpecialType9)的支持。以下是对代码的详细审查和改进意见: 1. 语法逻辑现状分析: 改进意见:
2. 代码质量现状分析: 改进意见:
3. 代码性能现状分析: 改进意见:
4. 代码安全现状分析: 改进意见:
总结这次改动主要是为了规范化命名和扩展对新硬件类型的支持,整体方向是正确的。主要的改进点在于:
代码示例(提取公共逻辑): // commonfunction.h
class Common {
public:
// ... 其他代码 ...
static bool isSpecialDisplayType() {
int type = specialComType;
return type == kSpecialType5 || type == kSpecialType6 ||
type == kSpecialType7 || type == kSpecialType9;
}
};
// DeviceMonitor.cpp
// 使用前
if (Common::specialComType == Common::kSpecialType5 || Common::specialComType == Common::kSpecialType6 ...)
// 使用后
if (Common::isSpecialDisplayType()) {
// ...
} |
Reviewer's GuideRefactors the SpecialComputerType enum to a consistent kSpecialTypeN naming scheme, replaces all legacy enum usages with the new names, and extends display/CPU logic to treat the new kSpecialType8 and kSpecialType9 variants appropriately. Updated class diagram for SpecialComputerType enum and its usagesclassDiagram
class Common {
+SpecialComputerType specialComType
+QString checkBoardVendorFlag()
+bool isHwPlatform()
}
class SpecialComputerType {
<<enumeration>>
Unknow = -1
NormalCom
kSpecialType1
kSpecialType2
kSpecialType3
kSpecialType4
kSpecialType5
kSpecialType6
kSpecialType7
kSpecialType8
kSpecialType9
}
class DeviceMonitor {
+void setInfoFromHwinfo(QMap_QString_QString mapInfo)
+TomlFixMethod setInfoFromTomlOneByOne(QMap_QString_QString mapInfo)
+QString getOverviewInfo()
+void loadBaseDeviceInfo()
+void loadOtherDeviceInfo()
+bool setMainInfoFromXrandr(QString info, QString rateInfo)
}
class DeviceCpu {
+void setCpuInfo(QMap_QString_QString mapLscpu, QMap_QString_QString mapCpuinfo)
}
class HWGenerator {
+void generatorDiskDevice()
}
class AppMain {
+int main(int argc, char argv)
}
Common ..> SpecialComputerType : uses
DeviceMonitor ..> Common : reads_specialComType
DeviceCpu ..> Common : reads_specialComType
HWGenerator ..> Common : reads_specialComType
AppMain ..> Common : reads_specialComType
Flow diagram for CPU frequency and disk interface logic with SpecialComputerTypeflowchart TD
subgraph CpuFrequencyAdjustment
A[Start setCpuInfo] --> B[Read Common.specialComType]
B --> C{Is specialComType in
kSpecialType5,
kSpecialType6,
kSpecialType7,
kSpecialType9?}
C -- Yes --> D[Replace 2.189 with 2.188 in m_Frequency]
D --> E[Replace 2189 with 2188 in m_MaxFrequency]
C -- No --> F[Leave CPU frequencies unchanged]
E --> G[Continue other CPU info processing]
F --> G[Continue other CPU info processing]
G --> H[End setCpuInfo]
end
subgraph DiskInterfaceOverride
I[Start generatorDiskDevice] --> J[Read Common.specialComType]
J --> K{Is specialComType kSpecialType2?}
K -- Yes --> L[Set Interface field to UFS]
K -- No --> M[Keep detected Interface]
L --> N[Continue disk generation]
M --> N[Continue disk generation]
N --> O[End generatorDiskDevice]
end
subgraph GpuPreGeneration
P[Start main] --> Q[Read Common.specialComType]
Q --> R{Is specialComType kSpecialType8
and no PCI graphics card?}
R -- Yes --> S[Call preGenerateGpuInfo]
R -- No --> T[Skip pre generation]
S --> U[Continue normal startup]
T --> U[Continue normal startup]
U --> V[End main startup phase]
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've left some high level feedback:
- There are several places now with long chains of
specialComType == kSpecialTypeXchecks (e.g., inDeviceMonitorandDeviceCpu); consider introducing small helper functions (likeisMonitorSpecialType()/isFrequencyAdjustedType()) or a set-based check to centralize this logic and reduce the chance of missing types in the future. - Renaming the enum values from domain-specific names (e.g.,
PGUW,KLVV) to generickSpecialTypeXreduces readability; if the semantic names are still relevant, consider either keeping them as aliases or adding comments/documentation on what eachkSpecialTypeXrepresents. - You’ve added handling for
kSpecialType9in multiple locations, but it’s not included incheckBoardVendorFlag()or other central dispatch points; double-check whether it should participate in those mappings as well so the behavior for the new type is consistently defined.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There are several places now with long chains of `specialComType == kSpecialTypeX` checks (e.g., in `DeviceMonitor` and `DeviceCpu`); consider introducing small helper functions (like `isMonitorSpecialType()` / `isFrequencyAdjustedType()`) or a set-based check to centralize this logic and reduce the chance of missing types in the future.
- Renaming the enum values from domain-specific names (e.g., `PGUW`, `KLVV`) to generic `kSpecialTypeX` reduces readability; if the semantic names are still relevant, consider either keeping them as aliases or adding comments/documentation on what each `kSpecialTypeX` represents.
- You’ve added handling for `kSpecialType9` in multiple locations, but it’s not included in `checkBoardVendorFlag()` or other central dispatch points; double-check whether it should participate in those mappings as well so the behavior for the new type is consistently defined.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: add-uos, max-lvs The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/forcemerge |
|
This pr force merged! (status: unstable) |
6db8731
into
linuxdeepin:develop/eagle
…ng convention
log: rename special computer type enums to consistent naming convention
task: https://pms.uniontech.com/task-view-385813.html
Change-Id: Id3da3d2b9ca530992ecddff69beb0da7a8fffcb6
Summary by Sourcery
Standardize special computer type enum names and extend handling for a new special type across device info, CPU, and monitor logic.
New Features:
Enhancements: