New repo in kdereview: plasma-phonebook
bshah at mykolab.com
Mon Feb 15 04:11:30 GMT 2021
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.
> 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
> 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
>  :(
> 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.
> 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
> 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`.
>  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.
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...
Size: 488 bytes
Desc: not available
More information about the kde-core-devel