chore: More logs for application#154
Merged
Merged
Conversation
Reviewer's GuideThis PR instruments the application with comprehensive qDebug() logging, adding entry/exit traces, parameter dumps, and decision-point logs across UI widgets, dialogs, D-Bus handler, and tree view components to improve runtime diagnostics. Class diagram for logging instrumentation in UI widgets and dialogsclassDiagram
class CreateVGWidget {
+initUi()
+initConnection()
+setScrollAreaAttribute()
+judgeDataEquality()
+showLoadingWidget()
+getVGName()
+getCurSelectPVData()
+onDoneButtonClicked()
+onCancelButtonClicked()
+updateData()
+createAvailableDiskData()
+resizeAvailableDiskData()
+onDiskItemClicked()
+updatePartitionData()
+onDiskCheckBoxStateChange()
+onPartitionCheckBoxStateChange()
+getCurSizeUnit()
+updateSelectedData()
+onDeleteItemClicked()
+onCurrentIndexChanged()
+onTextChanged()
+onVGCreateMessage()
+onUpdateUsb()
+getPVSize()
+getDevSize()
+getMaxSize()
+getMinSize()
+adjudicationPVMove()
<<Instrumented with qDebug logs>>
}
class PartitionWidget {
+initUi()
+topFrameSetting()
+midFrameSetting()
+botFrameSetting()
+partInfoShowing()
+recPartitionInfo()
+initConnection()
+sumValue()
+setSelectUnallocatesSpace()
+setAddOrRemResult()
+setRegValidator()
+maxAmountPrimReached()
+onShowSelectPathInfo()
+onShowTip()
+setEnable()
+setControlEnable()
+setLabelColor()
+onComboxCurTextChange()
+onComboxFormatTextChange()
+onJudgeLastPartition()
+onSliderValueChanged()
+onSetSliderValue()
+onTextChanged()
+onSetPartName()
+onAddPartition()
+onRemovePartition()
+onApplyButton()
+onRevertButton()
+onCancelButton()
+event()
+keyPressEvent()
<<Instrumented with qDebug logs>>
}
class DecryptDialog {
+initUi()
+initConnection()
+initData()
+setPasswordEditStatus()
+updateData()
+onPasswordEditTextChanged()
+onPasswordHintButtonClicked()
+onCancelButtonClicked()
+onButtonClicked()
+onDecryptMessage()
+onTimeOut()
+keyPressEvent()
<<Instrumented with qDebug logs>>
}
class VGInfoShowWidget {
+initUi()
+initConnection()
+setData()
<<Instrumented with qDebug logs>>
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @re2zero - I've reviewed your changes - here's some feedback:
- The PR adds a qDebug call in nearly every method and branch, which will flood the logs and hurt performance—please trim down to only key entry/exit or error events and consider using function-entry/exit macros instead of manual statements everywhere.
- Introduce QLoggingCategory (and qCDebug/qCWarning) so you can selectively enable or disable logs by module, rather than sprinkling qDebug unconditionally.
- Guard verbose debug logging behind a DEBUG or DEVELOPMENT build flag (or use Qt’s logging rules) to avoid shipping performance-heavy debug output in production.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The PR adds a qDebug call in nearly every method and branch, which will flood the logs and hurt performance—please trim down to only key entry/exit or error events and consider using function-entry/exit macros instead of manual statements everywhere.
- Introduce QLoggingCategory (and qCDebug/qCWarning) so you can selectively enable or disable logs by module, rather than sprinkling qDebug unconditionally.
- Guard verbose debug logging behind a DEBUG or DEVELOPMENT build flag (or use Qt’s logging rules) to avoid shipping performance-heavy debug output in production.
## Individual Comments
### Comment 1
<location> `application/widgets/infoshowwidget.cpp:661` </location>
<code_context>
QVector<double> size {usedSize, unusedSize};
m_infoWidget->setData(lvInfo, color, size, 1);
}
+ qDebug() << "InfoShowWidget::onHandleChangeTheme() called.";
}
</code_context>
<issue_to_address>
Debug output in theme change handlers may be unnecessary in production.
Frequent theme changes could lead to excessive logging. Please remove or conditionally compile this debug statement for production builds.
</issue_to_address>
### Comment 2
<location> `application/partedproxy/dmdbushandler.cpp:972` </location>
<code_context>
+ qDebug() << "CRYPT_ERR_CLOSE_FAILED for" << devPath;
break;
}
+ default:
+ qDebug() << "Unknown CRYPTError value:" << value;
+ break;
}
</code_context>
<issue_to_address>
Logging unknown enum values is useful, but consider error handling.
Consider handling unknown enum values explicitly, such as by notifying the user or implementing a fallback, in addition to logging.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
case CRYPTError::CRYPT_ERR_CLOSE_FAILED: {
failedMessage = tr("%1 failed to close the crypto map").arg(devPath);
qDebug() << "CRYPT_ERR_CLOSE_FAILED for" << devPath;
break;
}
=======
case CRYPTError::CRYPT_ERR_CLOSE_FAILED: {
failedMessage = tr("%1 failed to close the crypto map").arg(devPath);
qDebug() << "CRYPT_ERR_CLOSE_FAILED for" << devPath;
break;
}
default: {
qDebug() << "Unknown CRYPTError value:" << value;
failedMessage = tr("An unknown cryptographic error occurred for %1. Please contact support.").arg(devPath);
// Optionally, you could trigger a user notification here if your UI supports it.
break;
}
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `application/widgets/cylinderinfowidget.cpp:158` </location>
<code_context>
void CylinderInfoWidget::againVerify(int cylNumber)
{
+ qDebug() << "[CylinderInfoWidget] againVerify called with cylinder number:" << cylNumber;
m_curCheckCount = 0;
m_badSectorsCount = 0;
</code_context>
<issue_to_address>
Debug logging in UI update and loop-heavy code may cause performance issues.
Consider wrapping qDebug() statements in a debug flag or using a logging mechanism that can be toggled to avoid excessive output during widget iteration.
Suggested implementation:
```cpp
setLayout(mainLayout);
m_settings = new QSettings("/tmp/CheckData.conf", QSettings::IniFormat, this);
m_debugLoggingEnabled = false; // Default: debug logging off
if (m_debugLoggingEnabled) {
qDebug() << "[CylinderInfoWidget] CylinderInfoWidget initialized.";
}
}
```
```cpp
void CylinderInfoWidget::againVerify(int cylNumber)
{
if (m_debugLoggingEnabled) {
qDebug() << "[CylinderInfoWidget] againVerify called with cylinder number:" << cylNumber;
}
m_curCheckCount = 0;
m_badSectorsCount = 0;
m_scrollBar->hide();
m_isChanged = false;
m_startCylinder = m_settings->value("SettingData/BlockStart").toInt();
m_endCylinder = m_settings->value("SettingData/BlockEnd").toInt();
if (m_debugLoggingEnabled) {
qDebug() << "[CylinderInfoWidget] Initialized for re-verification, start cylinder:" << m_startCylinder << ", end cylinder:" << m_endCylinder;
}
QList<QObject *> lstCylinderWidget = m_widget->children();
if (m_debugLoggingEnabled) {
qDebug() << "[CylinderInfoWidget] Current cylinder widget count:" << lstCylinderWidget.count();
}
if (cylNumber <= lstCylinderWidget.count() - 1) { // 当将要重新检测的个数小于等于360时
```
You must add a `bool m_debugLoggingEnabled;` member to the `CylinderInfoWidget` class in the corresponding header file (`cylinderinfowidget.h`). Optionally, provide a public setter method to allow toggling debug logging at runtime:
```cpp
void setDebugLoggingEnabled(bool enabled) { m_debugLoggingEnabled = enabled; }
```
</issue_to_address>
### Comment 4
<location> `application/main.cpp:81` </location>
<code_context>
+ qDebug() << "executCmd called with command:" << strCmd;
QProcess proc;
#if QT_VERSION_MAJOR > 5
+ qDebug() << "QT_VERSION_MAJOR > 5, using split command start.";
QStringList list = strCmd.split(" ");
proc.start(list.at(0), QStringList() << list.at(1));
#else
+ qDebug() << "QT_VERSION_MAJOR <= 5, using direct command start.";
proc.start(strCmd);
#endif
</code_context>
<issue_to_address>
Command splitting in executCmd may not handle arguments with spaces or special characters.
Splitting the command by spaces and using only the first two elements can break when arguments contain spaces or special characters. Use a more reliable parsing method or pass the full command string to QProcess.
</issue_to_address>
### Comment 5
<location> `application/widgets/customcontrol/passwordinputdialog.cpp:175` </location>
<code_context>
void PasswordInputDialog::setTitleText(const QString &text)
{
+ qDebug() << "PasswordInputDialog::setTitleText called with text:" << text;
+ // This function seems to be empty, so no action needed other than logging.
+ qDebug() << "PasswordInputDialog::setTitleText finished";
}
</code_context>
<issue_to_address>
Logging in a function that performs no action may be unnecessary.
If this function is intended for future use, add a TODO comment. Otherwise, consider removing it to avoid unnecessary code.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
void PasswordInputDialog::setTitleText(const QString &text)
{
+ qDebug() << "PasswordInputDialog::setTitleText called with text:" << text;
+ // This function seems to be empty, so no action needed other than logging.
+ qDebug() << "PasswordInputDialog::setTitleText finished";
}
=======
void PasswordInputDialog::setTitleText(const QString &text)
{
// TODO: Implement setTitleText functionality if needed in the future.
}
>>>>>>> REPLACE
</suggested_fix>
### Comment 6
<location> `application/widgets/customcontrol/waterloadingwidget.cpp:58` </location>
<code_context>
int value = (m_waterProgress->value() + qrand() % 10);
+ qDebug() << "QT_VERSION_MAJOR <= 5, generated value:" << value;
#endif
value > 99 ? value = 99 : value;
m_waterProgress->setValue(value);
+ qDebug() << "Water progress value set to:" << value;
</code_context>
<issue_to_address>
The ternary expression does not assign the result; use an if-statement instead.
The current ternary expression does not update 'value' when 'value <= 99'. Use 'if (value > 99) value = 99;' for correct behavior and better readability.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Add more logs for application. 86.21% Log: More logs for application.
lzwind
approved these changes
Jul 7, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lzwind, re2zero 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add more logs for application. 86.21%
Log: More logs for application.
Summary by Sourcery
Add extensive qDebug logging throughout the application to record method entry points, key variable values, branching decisions and D-Bus interactions for better traceability.
Enhancements: