Fixed Header in 3 plugin pages: Run Command, Multimedia control, Remote input
ClosedPublic

Authored by brute4s99 on Nov 30 2018, 4:52 AM.

Details

Summary

Fixed Header in 3 plugins: Run Command, Multimedia control, Remote input

Test Plan

build the Android APK and analyze the Header in Run Command,
Remote input and Multimedia control plugins.

Diff Detail

Repository
R225 KDE Connect - Android application
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 5902
Build 5920: arc lint + arc unit
brute4s99 created this revision.Nov 30 2018, 4:52 AM
Restricted Application added a project: KDE Connect. · View Herald TranscriptNov 30 2018, 4:52 AM
Restricted Application added a subscriber: kdeconnect. · View Herald Transcript
brute4s99 requested review of this revision.Nov 30 2018, 4:52 AM

@sredman, I performed a code cleanup on the repository to remove unused code, please review

sredman requested changes to this revision.Nov 30 2018, 3:45 PM
sredman added inline comments.
src/org/kde/kdeconnect/Backends/BluetoothBackend/BluetoothPairingHandler.java
55 ↗(On Diff #46526)

Can you say why a lot of methods are no longer throwing? Was it unnecessary to begin with? In any case, it would be best if that change (exceptions) were separated from this change (header fixing)

This revision now requires changes to proceed.Nov 30 2018, 3:45 PM

Please change the strings only in the res/values/strings.xml. The translations are handled automatically

brute4s99 updated this revision to Diff 47052.Dec 7 2018, 6:00 PM
This comment was removed by brute4s99.
brute4s99 updated this revision to Diff 47057.Dec 7 2018, 6:12 PM
This comment was removed by brute4s99.
brute4s99 updated this revision to Diff 47307.Dec 10 2018, 6:18 PM

Updated the diff to reflect just the relevant changes for this commit.

Updated the diff to reflect just the relevant changes for this commit.

Looks good from this point of view. Thanks!

@nicolasfella, do you know whether changing the names of the strings in strings.xml will mess up the translators? If so, we should probably not touch those. It will make our code a little strange but it would save a lot of hassle on their end.

Updated the diff to reflect just the relevant changes for this commit.

Looks good from this point of view. Thanks!

@nicolasfella, do you know whether changing the names of the strings in strings.xml will mess up the translators? If so, we should probably not touch those. It will make our code a little strange but it would save a lot of hassle on their end.

I don't know for sure if it will cause problems, but just to be sure I'd not change the translated files

Since I've changed the identifier for the strings in every translation, it will not give any errors. I can post some Screenshots with other languages if you'd like!

Since I've changed the identifier for the strings in every translation, it will not give any errors. I can post some Screenshots with other languages if you'd like!

The thing I am woirred about is not the app having trouble, but I am worried this might cause trouble for the translation team. I don't know if they use the Android Studio translator. Since they also work on all of the other Qt projects in KDE, I expect that they might not.

I just don't know, and I would rather avoid risking a headache for them, since their job is already a hassle. If you would like to go ask them how their workflow looks and whether changing the names of already-translated strings will or will not cause a problem, I'm sure they would be happy to let you know!

Since I've changed the identifier for the strings in every translation, it will not give any errors. I can post some Screenshots with other languages if you'd like!

The thing I am woirred about is not the app having trouble, but I am worried this might cause trouble for the translation team. I don't know if they use the Android Studio translator. Since they also work on all of the other Qt projects in KDE, I expect that they might not.

I just don't know, and I would rather avoid risking a headache for them, since their job is already a hassle. If you would like to go ask them how their workflow looks and whether changing the names of already-translated strings will or will not cause a problem, I'm sure they would be happy to let you know!

It all depends on the deadlines. If the release date (which should be published on phabricator.kde.org/calendar or at least announced on kde-i18n-doc@ in advance, even if it's not precise) is not in one or two days, then it's fine to do changes.
The policy is different when a project maintain stable branches. In that case, an exception should be requested on kde-i18n-doc@ before touching strings in the stable branch. But I think it's not the case here.

That said, the translation strings are automatically extracted from the code every 24 hours, so please don't touch anything inside res/valus-<id>. Just change the code.

brute4s99 added a comment.EditedDec 11 2018, 9:51 AM

Understood, @Itoscano. Thank you for the prompt reply!

brute4s99 updated this revision to Diff 47341.Dec 11 2018, 11:15 AM

Final patch with original identifiers in strings.xml

Sorry, this is going to feel like a tease. This is really good, and I'm sorry I'm so picky :)

Make sure to remove the now-unused strings from *just* strings.xml, not the translation files. For instance, the string "@string/remote_control" is no longer used

brute4s99 updated this revision to Diff 47576.Dec 14 2018, 6:12 PM

Removed the line, @sredman . Does it look OK?

sredman accepted this revision.Dec 14 2018, 10:09 PM
This revision is now accepted and ready to land.Dec 14 2018, 10:09 PM
This revision was automatically updated to reflect the committed changes.