chore: clean up verbose debug logs in device detection#103
chore: clean up verbose debug logs in device detection#103deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
- Remove step-by-step debug logs in ListUsbDrives() - Consolidate device detection output to single summary line - Clean up debug logs for Windows, Linux and Mac platforms Log: Remove redundant debug logs during USB device enumeration
Reviewer's guide (collapsed on small PRs)Reviewer's GuideCleans up verbose USB device detection logging across platforms by removing step-by-step debug output in the low-level enumeration function and consolidating reporting into a single summary log line in the device monitor. Sequence diagram for consolidated USB device detection loggingsequenceDiagram
participant QTimer
participant DeviceMonitor
participant Utils
participant OS
participant Logger
QTimer->>DeviceMonitor: timeout()
DeviceMonitor->>Utils: ListUsbDrives()
Utils->>OS: enumerate_usb_devices()
OS-->>Utils: deviceList_raw
Utils-->>DeviceMonitor: deviceList
DeviceMonitor->>DeviceMonitor: collect device paths into devices
DeviceMonitor->>Logger: qDebug Detected deviceList.size() USB devices: devices
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto reviewGit Diff 代码审查报告整体评价本次代码修改的主要目的是清理 详细审查1. devicemonitor.cpp 修改修改内容:
优点:
建议:
2. utils.cpp 修改修改内容:
优点:
建议:
具体改进建议1. 语法和逻辑代码语法正确,逻辑清晰,没有明显问题。 2. 代码质量devicemonitor.cpp: // 原代码
for (int i = 0; i < list.size(); i++)
{
devices << list.at(i).path;
}
qDebug() << "Detected" << list.length() << "USB devices: " << devices;
// 建议改进
for (const auto &device : list) {
devices << device.path;
}
qDebug() << "Detected" << list.size() << "USB devices:" << devices.join(", ");3. 性能优化
4. 安全性
总结本次修改主要是清理调试日志,提高了代码简洁性和执行效率。建议考虑以下几点进一步改进:
总体而言,修改方向正确,代码质量良好,没有明显的安全问题。 |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/libdbm/util/utils.cpp:519` </location>
<code_context>
QMap<QString, DeviceInfo> dfDeviceInfos = CommandDfParse();
QMap<QString, DeviceInfo> lsblkDeviceInfos = CommandLsblkParse();
@@ -518,13 +514,7 @@ QList<DeviceInfo> ListUsbDrives()
removeDevice.insert(filePath, fileName);
}
}
- qDebug() << "Step 4: Filter devices - Found" << removeDevice.size() << "USB/MMC devices from" << usbfileinfoL.size() << "total devices";
- for (auto it = removeDevice.begin(); it != removeDevice.end(); ++it) {
- qDebug() << " Device path:" << it.key() << "Device name:" << it.value();
- }
-
- qDebug() << "Step 5: Traverse device partitions and filter eligible devices";
int processedCount = 0;
int validCount = 0;
</code_context>
<issue_to_address>
**suggestion:** Consider removing or repurposing processedCount/validCount now that they’re only used for logging.
Now that the related logging is gone, `processedCount` and `validCount` are maintained only for unused debug output, which adds noise and potential confusion. If they’re not used elsewhere, consider removing them or giving them a clear functional role (for example, asserting that `deviceList.size()` equals `validCount`).
Suggested implementation:
```cpp
```
```cpp
```
```cpp
```
1. If there are any remaining references to `processedCount` or `validCount` (e.g., in `qDebug()` or assertions) later in `ListUsbDrives()`, those should be removed or refactored as well.
2. If you do want a functional role for such counters, reintroduce them with a clear purpose (for instance, `int validDevices = deviceList.size(); Q_ASSERT(validDevices == deviceList.size());`) instead of maintaining separate counters throughout the loop.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| qDebug() << "Step 5: Traverse device partitions and filter eligible devices"; | ||
| int processedCount = 0; | ||
| int validCount = 0; |
There was a problem hiding this comment.
suggestion: Consider removing or repurposing processedCount/validCount now that they’re only used for logging.
Now that the related logging is gone, processedCount and validCount are maintained only for unused debug output, which adds noise and potential confusion. If they’re not used elsewhere, consider removing them or giving them a clear functional role (for example, asserting that deviceList.size() equals validCount).
Suggested implementation:
- If there are any remaining references to
processedCountorvalidCount(e.g., inqDebug()or assertions) later inListUsbDrives(), those should be removed or refactored as well. - If you do want a functional role for such counters, reintroduce them with a clear purpose (for instance,
int validDevices = deviceList.size(); Q_ASSERT(validDevices == deviceList.size());) instead of maintaining separate counters throughout the loop.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: itsXuSt, lzwind 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) |
Log: Remove redundant debug logs during USB device enumeration
Summary by Sourcery
Clean up USB device detection logging to reduce verbosity while preserving a concise summary of detected devices.
Enhancements: