chore: [translate] support lupdate auto#556
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom Nov 28, 2025
Merged
chore: [translate] support lupdate auto#556deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
support lupdate auto Log: support lupdate auto Bug: https://pms.uniontech.com/task-view-368603.html
Reviewer's GuideAdds a private helper method to DeviceBaseInfo that collects all device-related user-visible strings into tr() calls so Qt lupdate can automatically pick them up for translation, without changing runtime behavior. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review我来对这个代码变更进行审查：
a) 代码组织：
b) 性能优化：
c) 安全性：
d) 可维护性：
// 在 DeviceInfo.h 中
private:
static void generatorTranslate();
// 在 DeviceInfo.cpp 中
void DeviceBaseInfo::generatorTranslate()
{
// 这些字符串用于设备信息的国际化显示
static const QStringList translationStrings = {
tr("Core(s)"),
tr("Processor"),
// ... 其他翻译字符串
tr("Processor")
};
// 避免编译器警告，实际不需要使用这些字符串
Q_UNUSED(translationStrings);
}
总的来说，这个变更实现了其预期功能，但在代码组织和可维护性方面还有改进空间。 |
lzwind
approved these changes
Nov 28, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: add-uos, 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 |
![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:
- The private
generatorTranslate()method is never called; consider usingQT_TRANSLATE_NOOP/QT_TR_NOOPmacros or a dedicated translation context instead of a dead member function to register strings forlupdate. - Inside
generatorTranslate(),translationStringsis constructed but never used, which may trigger warnings; either mark it withQ_UNUSED(translationStrings)or refactor to a compile-time only mechanism for translation extraction.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The private `generatorTranslate()` method is never called; consider using `QT_TRANSLATE_NOOP`/`QT_TR_NOOP` macros or a dedicated translation context instead of a dead member function to register strings for `lupdate`.
- Inside `generatorTranslate()`, `translationStrings` is constructed but never used, which may trigger warnings; either mark it with `Q_UNUSED(translationStrings)` or refactor to a compile-time only mechanism for translation extraction.
## Individual Comments
### Comment 1
<location> `deepin-devicemanager/src/DeviceManager/DeviceInfo.cpp:1178-1180` </location>
<code_context>
}
-
+//只是为了lupdate自动增加翻译而占位
+void DeviceBaseInfo::generatorTranslate()
+{
+ QStringList translationStrings;
+ translationStrings \
+ << tr("Core(s)") \
</code_context>
<issue_to_address>
**suggestion:** Avoid allocating an unused QStringList when only triggering lupdate; use simple tr() calls instead.
Given this is only for lupdate, you can replace the list construction with a simple sequence of `tr("...");` calls (optionally wrapped in `if (false)` or `#if 0`) so lupdate still sees the strings without runtime work. If you do keep the `QStringList`, consider adding `Q_UNUSED(translationStrings)` to avoid unused-variable warnings.
Suggested implementation:
```cpp
//只是为了lupdate自动增加翻译而占位
void DeviceBaseInfo::generatorTranslate()
{
// Only for lupdate: ensure these strings are picked up without doing runtime work
if (false) {
tr("Core(s)");
tr("Processor");
tr("ACL MTU");
tr("Address");
tr("Alias");
tr("ansiversion");
tr("Application");
tr("Architecture");
tr("Array Handle");
tr("Asset Tag");
tr("Auto Negotiation");
```
The original code snippet is truncated; there are likely additional `<< tr("...") \` entries following `"Auto Negotiation"`.
You should:
1. Continue the pattern inside the `if (false) { ... }` block, adding a `tr("...");` line for each of the remaining strings that were originally in the `QStringList`.
2. Ensure the `if (false)` block is properly closed with a `}` and that the `generatorTranslate()` function itself is closed with a final `}` after the block.
This will keep lupdate behavior intact while avoiding allocation of an unused `QStringList` and eliminating unused-variable warnings.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+1178
to
+1180
| void DeviceBaseInfo::generatorTranslate() | ||
| { | ||
| QStringList translationStrings; |
There was a problem hiding this comment.
suggestion: Avoid allocating an unused QStringList when only triggering lupdate; use simple tr() calls instead.
Given this is only for lupdate, you can replace the list construction with a simple sequence of tr("..."); calls (optionally wrapped in if (false) or #if 0) so lupdate still sees the strings without runtime work. If you do keep the QStringList, consider adding Q_UNUSED(translationStrings) to avoid unused-variable warnings.
Suggested implementation:
//只是为了lupdate自动增加翻译而占位
void DeviceBaseInfo::generatorTranslate()
{
// Only for lupdate: ensure these strings are picked up without doing runtime work
if (false) {
tr("Core(s)");
tr("Processor");
tr("ACL MTU");
tr("Address");
tr("Alias");
tr("ansiversion");
tr("Application");
tr("Architecture");
tr("Array Handle");
tr("Asset Tag");
tr("Auto Negotiation");
The original code snippet is truncated; there are likely additional << tr("...") \ entries following "Auto Negotiation".
You should:
- Continue the pattern inside the
if (false) { ... }block, adding atr("...");line for each of the remaining strings that were originally in theQStringList. - Ensure the
if (false)block is properly closed with a}and that thegeneratorTranslate()function itself is closed with a final}after the block.
This will keep lupdate behavior intact while avoiding allocation of an unusedQStringListand eliminating unused-variable warnings.
Contributor
Author
|
/merge |
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.
support lupdate auto
Log: support lupdate auto
Bug: https://pms.uniontech.com/task-view-368603.html
Summary by Sourcery
Enhancements: