New repo in kdereview: plasma-phonebook

[Bhushan Shah]

Hello,

On Mon, Jan 25, 2021 at 02:18:51PM +0100, Harald Sitter wrote:
> 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.

https://invent.kde.org/plasma-mobile/plasma-phonebook/commit/98d768493ab7ab1360d454983bb99e339751d91c

> 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

https://invent.kde.org/plasma-mobile/plasma-phonebook/-/commit/a89acbfc9e4227a7cf0d91f5ecdee8cf105710f8

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

https://invent.kde.org/plasma-mobile/plasma-phonebook/-/commit/e7c1e54d71d0c3cdd36950d0f8b326df44aca115

> 
> Many files don't have license info, see point 15 of the license policy
> [1] :(
> 
> Actual qml **source** files have no license info!

Licensing is being sorted out as I write this email. I will reach out to
Fabian, original author of some of these files for clarification

> 
> DetailListItem.qml isn't used.

https://invent.kde.org/plasma-mobile/plasma-phonebook/-/commit/4add44d12f9d43ab4f8eba181f3ad5b681429dc6

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

This needs looking into.

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

https://invent.kde.org/plasma-mobile/plasma-phonebook/-/commit/27a94cdda11ecc5b6ed0269917a2643d5f632518

> 
> # 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`.

https://invent.kde.org/plasma-mobile/plasma-phonebook/-/commit/1007e7b2f0c916bdf9f74b2ae63a8d494cd807c9

> 
> [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.
> 
> 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.
> 
> HS
> 




-- 
Bhushan Shah
http://blog.bshah.in
IRC Nick : bshah on Freenode
GPG key fingerprint : 0AAC 775B B643 7A8D 9AF7 A3AC FE07 8411 7FBC E11D
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20210215/b5214d7d/attachment.sig>