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

Previous Next

Full log

View this message in rfc822 format

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

Hi,

muradm <mail <at> muradm.net> writes:

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

OK; so the new version will need some fiddling from users to just build
with their current config?  Or are these expected to be rare edge cases?

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

OK.

[...]

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

OK, I must have gotten confused by the messy diff.

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

See (info '(texinfo) @table):

   Write the ‘@table’ command at the beginning of a line, after a blank
   line, and follow it on the same line with an argument that is an
   'indicating' command, such as ‘@code’, ‘@samp’, ‘@var’, ‘@option’, or
   ‘@kbd’ (*note Indicating::).  This command will be applied to the text
   in the first column.  For example, ‘@table @code’ will cause the text
   in the first column to be output as if it had been the argument to a
   ‘@code’ command.

I think the 'indicating' command argument to @table is required, so
there is no default value.

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

I meant perhaps waiting 1 second here is no longer necessary?  1 s is
not much at all.  Perhaps added during debug/development of the service
and forgotten?  I'd trying taking it out and see.

-- 
Thanks,
Maxim

Previous Next

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