Subject: Re: [bug#78188] [PATCH v2] services: kwallet: New service.

From: Sergio Pastor Pérez <[email-masked]>
Date: Fri, 02 May 2025 09:24:06 +0200

Maxim Cournoyer <[email-masked]> writes:
> Sergio Pastor Pérez <[email-masked]> writes:
>> +@defvar kwallet-service-type
>> +This is the type of the service that adds the
>> +@uref{https://invent.kde.org/plasma/kwallet-pam, KWallet keyring}.  Its
>> +value is a @code{kwallet-configuration} object (see below).  Note that,
>> +contrary to @code{gnome-desktop-service-type},
>> +@code{plasma-desktop-service-type} does not include this service.
>
> Does gnome-desktop-service-type include the kwallet-service-type?  I
> wouldn't think so.  You probably meant to say it "doesn't include a
> wallet service in its default configuration." ?  Is this the normal
> expectation for the KDE desktop?  I'd assume it comes with kwallet
> pre-configured, if using Fedora for example.  If it does, we should
> probably do so to avoid breaking users expectations.  I've recently made
> the adjustment in GNOME to have the GNOME keyring unlocked by default
> for the GNOME desktop, as that's what users expect.

I think I made a mistake with the wording. What I meant is that, while
the `gnome-desktop-service-type' includes a keyring field which provides
the functionality of `gnome-keyring-service-type' making it necessary
for a `gnome-desktop-service-type' user to add a
`gnome-keyring-service-type' entry to their service list; the
`plasma-desktop-service-type' does not provide this functionality by
default. Which contrary to what the user may expect, they will need to
add the `kwallet-service-type' to their list of services even if they
are users of the `plasma-desktop-service-type'.

In the v2 of this patch series I've rewrote that bit to make it more
clear. Let me know what you think.

>> +;; TODO: consider integrating service in `<plasma-desktop-configuration>' as
>> +;; done in `<gnome-desktop-configuration>'. This requires rewritting the
>> +;; `<plasma-desktop-service-type>' as done for `<gnome-desktop-service-type>'.
>
> Ah, I see my comment above is acknowledged here as a TODO.  I'd
> encourage you to pursue that next!

This will require a major refactor of the `plasma-desktop-service-type'
as was done for the `gnome-desktop-service-type' to make it
modular. Once we get this merged I will try to join the kde team to help
with the refactor.

> Otherwise it LGTM.  Could you please send a v2?

Sure, aside for the things I've mentioned I've also corrected all the
typos you noticed.

Thanks for the review!

Best regards,
Sergio.