Add tooltips to Umbrello and change the strings to i18n().
BUG: 83444
FIXED-IN: 2.31.0 (KDE release 20.04.0)
Details
Details
Diff Detail
Diff Detail
- Repository
- R139 Umbrello
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Comment Actions
Thanks for fixing the i18n() issue.
As it was mentioned by @ltoscano , it would be good for bug handling if there is a separate line (not a mention of the number) with the command for our scripts to close the bug. That's it, literally:
BUG: 83444
Comment Actions
Thanks in advance for fixing these minor issues.
| umbrello/umlwidgets/componentwidget.cpp | ||
|---|---|---|
| 47 ↗ | (On Diff #76191) | Missing space after "physical". |
| umbrello/umlwidgets/objectnodewidget.cpp | ||
| 202 ↗ | (On Diff #76191) | Typo:
should be
|
| 210 ↗ | (On Diff #76191) | Typo:
should be
|
| 213 ↗ | (On Diff #76191) | Typo:
should be
|
| 216 ↗ | (On Diff #76191) | Typo:
should be
|
| 219 ↗ | (On Diff #76191) | Missing space after "creates". |
Comment Actions
There is also a line for specifying the version with which this extension request was added for the news and changelog (e.g https://umbrello.kde.org/changelog.php?20.04
FIXED-IN: 2.31.0 (KDE release 20.04.0)
If the patch for this check request is added to git before 3/19/20, it will actually be available with version 2.30.80, but I suspect that more time will be needed to translate the strings
See https://community.kde.org/Schedules/release_service/20.04_Release_Schedule for more details about scheduling,
Comment Actions
And one more typo.
| umbrello/umlwidgets/artifactwidget.cpp | ||
|---|---|---|
| 40 ↗ | (On Diff #76191) | Typo: pro-cess -> process |
Comment Actions
The last bunch of minor issues found while translating this. Thanks in advance for fixing them and for your patience.
| umbrello/umlwidgets/associationline.cpp | ||
|---|---|---|
| 621 ↗ | (On Diff #76191) | Should be "Sequence Self Message" |
| umbrello/umlwidgets/combinedfragmentwidget.cpp | ||
| 281 ↗ | (On Diff #76191) | Missing space after "invalid." |
| umbrello/umlwidgets/statewidget.cpp | ||
| 442 ↗ | (On Diff #76191) | Unfinished sentence. |
| umbrello/umlwidgets/associationline.cpp | ||
|---|---|---|
| 155 ↗ | (On Diff #76191) | unrelated change, please omit. |
| 585 ↗ | (On Diff #76191) | A tooltip is missing here |
| 737–738 ↗ | (On Diff #76191) | A tooltip is missing here |
| umbrello/umlwidgets/classifierwidget.cpp | ||
| 1015 | This change also has nothing to do with this issue. While it's good to fix empty lines or indentations, this should be done in a separate commit. This is also true for other similar places. | |
| umbrello/umlwidgets/objectnodewidget.cpp | ||
| 181 ↗ | (On Diff #76191) | This change has nothing to do with the bug this patch is intended for, so please omit. Instead, the actual problem, namely the missing support for translations, should be fixed. (see https://bugs.kde.org/show_bug.cgi?id=418150) this patch is intended for. |
| 335–336 ↗ | (On Diff #76191) | same as mentioned above |
| 350 ↗ | (On Diff #76191) | dito |
| umbrello/umlwidgets/portwidget.cpp | ||
| 45–48 ↗ | (On Diff #76191) | Please remove this line, git log already indicates that it has been removed |
| umbrello/umlwidgets/statewidget.cpp | ||
| 133–139 ↗ | (On Diff #76191) | Does is a unrelated change without any change, please omit. |
| umbrello/umlwidgets/associationline.cpp | ||
|---|---|---|
| 527 ↗ | (On Diff #76191) | I am not happy with the extensive change in the order of the cases, because it makes it very difficult to check what exactly has changed and if something has broken. |
| umbrello/umlwidgets/combinedfragmentwidget.cpp | ||
| 240 ↗ | (On Diff #76191) | Please order alphabetically by case. |
| umbrello/umlwidgets/messagewidget.cpp | ||
| 869 ↗ | (On Diff #76191) | Please sort alphabetically by case. |
| umbrello/umlwidgets/notewidget.cpp | ||
| 146 ↗ | (On Diff #76191) | Please sort alphabetically by case. |
| umbrello/umlwidgets/objectnodewidget.cpp | ||
| 201 ↗ | (On Diff #76191) | dito, sort alphabetically |
| umbrello/umlwidgets/signalwidget.cpp | ||
| 229 ↗ | (On Diff #76191) | Either use an if else chain or a switch statement and please order alphabetically |
| umbrello/umlwidgets/statewidget.cpp | ||
| 425 ↗ | (On Diff #76191) | Please sort alphabetically |
Comment Actions
The requested changes have been made, I will run a diff and double check then submit the changes on 2/25.
Thanks for your patience and comments.
Comment Actions
Please update the patch with the changes applied
then submit the changes on 2/25.
Please wait until I can inspect the updated patch and set this review in the "ready to land" state
Thanks for you work, which is appreciated
Comment Actions
PS: phabricator is not good at managing reviews that consist of multiple patches. A better way to do this is to create a fork from the project https://invent.kde.org/kde/umbrello, push the patches into a separate branch and create a merge request.
See https://invent.kde.org/kde/umbrello/-/merge_requests/3 for an example.