New repo in kdereview: plasma-phonebook

[Albert Astals Cid]

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
> 
>