sync: from linuxdeepin/dtkdeclarative #291
Merged
Conversation
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
Contributor
Author
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: deepin-ci-robot 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 |
Reviewer's GuideThis PR synchronizes upstream changes from linuxdeepin/dtkdeclarative, introducing Qt6 compatibility via conditional std::as_const usage, refining function declarations and default initializations, adjusting build definitions, correcting platform attribute checks, and removing dead code across multiple modules. Class diagram for updated SettingsContainer and SettingsGroup iteration logicclassDiagram
class SettingsContainer {
-QVector<SettingsGroup *> m_groups
+QVector<SettingsGroup *> groupList() const
+bool groupVisible(const QString &key) const
+void resetSettings()
}
class SettingsGroup {
-QList<SettingsGroup *> m_children
-QList<Option *> m_options
+void setConfig(QObject *config)
+void setBackground(QQmlComponent *background)
}
SettingsContainer "1" *-- "*" SettingsGroup
SettingsGroup "1" *-- "*" Option
Class diagram for WaterPopAttribute method signature changesclassDiagram
class WaterPopAttribute {
+qreal sizeRatio() const
+qreal yOffset() const
+void setYOffset(qreal offset)
+signal xSpeedChanged()
}
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 there - I've reviewed your changes - here's some feedback:
- There are numerous repeated #if QT_VERSION checks to switch between qAsConst and std::as_const—consider introducing a compatibility alias or helper to avoid redundant code blocks and improve readability.
- The D_IGNORE_DEPRECATIONS compile definition is added in multiple CMakeLists (src and qt6/src); you may want to centralize it in a single location to ensure consistency and ease maintenance.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There are numerous repeated #if QT_VERSION checks to switch between qAsConst and std::as_const—consider introducing a compatibility alias or helper to avoid redundant code blocks and improve readability.
- The D_IGNORE_DEPRECATIONS compile definition is added in multiple CMakeLists (src and qt6/src); you may want to centralize it in a single location to ensure consistency and ease maintenance.
## Individual Comments
### Comment 1
<location> `src/private/dbackdropnode.cpp:148` </location>
<code_context>
}
static DataManagerPointer<Derive> resolve(const DataManagerPointer<Derive> &other, QQuickWindow *owner) {
- static_assert(&Derive::metaObject);
Q_ASSERT(owner);
if (other && other->owner() == owner)
</code_context>
<issue_to_address>
Removing static_assert may reduce compile-time type safety.
Consider if removing this compile-time check could allow incorrect template instantiations, leading to runtime errors. If so, evaluate whether an alternative validation is necessary.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
|
|
||
| static DataManagerPointer<Derive> resolve(const DataManagerPointer<Derive> &other, QQuickWindow *owner) { | ||
| static_assert(&Derive::metaObject); |
There was a problem hiding this comment.
issue: Removing static_assert may reduce compile-time type safety.
Consider if removing this compile-time check could allow incorrect template instantiations, leading to runtime errors. If so, evaluate whether an alternative validation is necessary.
Synchronize source files from linuxdeepin/dtkdeclarative. Source-pull-request: linuxdeepin/dtkdeclarative#519
deepin-ci-robot
force-pushed
the
sync-pr-519-nosync
branch
from
a05a88f to
b8f1d01
Compare
Contributor
Author
deepin pr auto review根据提供的代码差异，我来对代码进行审查，并提出改进意见： 1. Qt版本兼容性改进优点：
改进建议：
2. 代码质量和可维护性优点：
改进建议：
3. 代码安全性优点：
改进建议：
4. 性能优化优点：
改进建议：
5. 其他建议
总体来说，这些改动主要是为了提高Qt6兼容性，代码质量整体良好。主要的改进空间在于减少重复代码、提高安全性，以及优化性能。建议在后续开发中继续保持对版本兼容性的重视，并逐步完善代码的可维护性。 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Synchronize source files from linuxdeepin/dtkdeclarative.
Source-pull-request: linuxdeepin/dtkdeclarative#519
Summary by Sourcery
Sync source files from linuxdeepin/dtkdeclarative and adapt code for Qt6 compatibility, deprecation suppression, and minor cleanup across containers, controls, window management, loader, and examples.
Bug Fixes:
Enhancements:
Build:
Tests:
Chores: