El dilluns, 25 de gener de 2021, a les 14:18:51 CET, Harald Sitter va escriure: > On 25.01.21 06:48, Bhushan Shah wrote: > > Hello everyone! > > > > I am back with more Plasma Mobile related repositories in kdereview. I > > want to move plasma-phonebook in kdereview. plasma-phonebook is kirigami > > based phonebook application, it uses kpeople backends like kpeoplesink, > > kpeoplevcard, kpeople akonadi backend etc to fetch and store contacts on > > your system. > > I am pretty sure l10n isn't working. I can't see the translations domain > set anywhere. > > On the subject of strings I'd suggest checking them all for HIG > compliance. In main.qml alone all strings are wrong as titles and > buttons are supposed to use Title Capitalization > https://hig.kde.org/style/writing/capitalization.html > > The desktop file seems to be lacking an actual main category (it's in > the lost&found section in the launcher for me) > > What's the use case for the unregistered x-plasma-phonebook mimetype > (registered for by the desktop file)? Perhaps the more relevant question > is why would it should be unregistered instead of inside our vendor tree > (vnd.kde.phone.book or some such)? > > Many files don't have license info, see point 15 of the license policy > [1] :( > > Actual qml **source** files have no license info! > > DetailListItem.qml isn't used. > > Are we quite sure that hardcoding colors is the way to go? :O > > Header.qml actually has a crash for me because ColorOverlay is inside a > FastBlur that is both parent and source (qt docs about source: "Note: It > is not supported to let the effect include itself, for instance by > setting source to the effect's parent.") > > Similarly the FastBlur looks to be falling into that trap of > parent===source, albeit not crashing for me. > > Spelling is inconsistent. The desktop file spells it "Phone Book" the > appstream file spells it "Plasma Phonebook". > > .appdata.xml is a legacy suffix, you might want to use .metainfo.xml > instead. > > On the subject of the appstream file. Missing > <project_group>KDE</project_group> :( > > # Probably a bit nitpicky > > Failing to store contact changes in AddContactPage lacks actual UI > backing, it merely qwarns. > > Various single argument ctors aren't tagged `explicit`. > > [1] https://community.kde.org/Policies/Licensing_Policy > > > Another thing I want to discuss is, branding, currently it is named as > > Plasma Phonebook which is bit awkward because Plasma is not related to > > app at all and even android port for app exists. Maybe we should name > > this just phonebook? > > Is this about the repo/tarball name or the display string? > > The former already lacks the plasma prefix in the .desktop file, I'd > argue the appstream name should do the same. System Settings is a > precedent for something similar.
Yeah i think that as a user visible name "it makes sense", like the app to read SMS in my phone is just called "Messages", because well that's what it does, no need for a fancy name. > As for tarballs: some distros get grumpy over overly generic source > names, so while I think phonebook is fine it may be less fine for > distros. It probably matters little what the repo/tarball is called though. I would prefer something more "qualified" than phonebook as an internal codename plamo-phonebook? kpeople-phonebook? phonebook-kpeople? org.kde.phonebook? E.g. installing a phonebook.mo in the the global translations folder (when for the desktop) is a bit of a somewhat nasty/agressive move in my opinion. Cheers, Albert > > HS > >
