GNU bug report logs - #75270
[PATCH 0/3] services: greetd: Improve greeter configurations.

Previous Next

Full log

Message #131 received at 75270 <at> debbugs.gnu.org (full text, mbox):

From: muradm <mail <at> muradm.net>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: ludo <at> gnu.org, 75270 <at> debbugs.gnu.org, pelzflorian <at> pelzflorian.de
Subject: Re: [bug#75270] [PATCH v4 1/3] services: greetd: Improve greeter
 configurations.
Date: Mon, 03 Feb 2025 02:17:44 +0300

[Message part 1 (text/plain, inline)]

Hi,

Skipped comments are ok or done. Commenting most notable only.

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:

> You are changing the public API rather drastically.  You need to 
> come up
> with a mechanism that will warn & automatically rewrite the 
> legacy form
> to the new form in the meantime, which I'm afraid won't be 
> trivial given
> the disruptive overhaul.  There are examples among other 
> services; it
> typically involves defining a man-in-the-middle field sanitizer 
> that
> takes the old value, updates it to the new expected one while 
> emitting a
> deprecation warning.

Done. It should be noted that it is impossible to cover all 
possible cases
to provide solid backward compatibility. Uses of file-like objects
increase variations.

> I was wondering why 'greetd-agreety-session' was moved around; 
> it makes
> your change more confusing to review.  Also, please use double 
> space
> between sentences, which is conventional in Guix.

Unfortunately, when greetd-service-type first created, it was not 
clear
for me how to make things clean. Now that cleaning requires 
somewhat
complex refactoring.

> s/user/users/, s/command/commands/, or 'a stable shell command'. 
> I'm
> not sure what 'stable' means in this context.

Changed stable to POSIX. Idea was to specify something "working 
always".

> I'd reword to: Typically, a <greetd-user-session> record 
> instance is
> used, but file-like objects are also valid.

This one is changed. Having configurable <greetd-user-session>, I 
don't
think that it is worth supporting file-like there.

> I'm not convinced the greetd-wlgreet-color API change is worth 
> the
> trouble (especially given you need to make it 
> backward-compatible with a
> custom record field accessor that checks what it gets and adapts 
> it to
> the new expected format, emitting a deprecation warning in the 
> process).

I was not either. Dropped greetd-wlgreet-color.

> Is that new default worth it/better?  It'll create a not so 
> useful empty
> file in the store.

I didn't introduce that default, but documented it. With v5 
default fixed.

> Looks like all the tables can use '@table @code' instead of 
> @asis, then
> you can remove all the @code decorators for each items.  This 
> could be
> done in a prior commit.

I didn't find any use of '@table @code' which document default 
values.

>> -           (sleep 1) ;give seatd/logind some time to start up
>
> That's a suspicious line which already existed.  It looks 
> fragile.  Is
> it really necessary?

Unfortunately, there is no good/easy way to conditionally depend
on elogind or seatd. greetd-service-type and agreety greeter do 
not require
seat, only sway based greeters may/will want it.

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.