GNU bug report logs - #57960
[PATCH core-updates] gnu: libxkbcommon: Switch to gexp; adjust style.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 57960 in the body.
You can then email your comments to 57960 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Felix Lechner <felix.lechner <at> lease-up.com>
To: guix-patches <at> gnu.org
Subject: [PATCH core-updates] gnu: libxkbcommon: Switch to gexp; adjust style.
Date: Tue, 20 Sep 2022 12:23:34 -0700

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

Hi,

This modernizes the package declaration for libxkbcommon by using
G-expressions. Also, input folders are found via their expected
relative paths (i.e. the modern way) instead of the input label. The
latter was an issue when the official xkeyboard-config was replaced
with a customized package holding additional keyboard layouts.

Ultimately reaches 3435 packages and thus belongs into core-updates.

Kind regards
Felix Lechner

[0001-gnu-libxkbcommon-Switch-to-gexp-adjust-style.patch (text/x-patch, attachment)]

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

From: Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at>
To: Felix Lechner <felix.lechner <at> lease-up.com>, 57960 <at> debbugs.gnu.org
Subject: Re: [PATCH core-updates] gnu: libxkbcommon: Switch to gexp; adjust
 style.
Date: Wed, 21 Sep 2022 11:03:15 +0200

Am Dienstag, dem 20.09.2022 um 12:23 -0700 schrieb Felix Lechner:

> +              (method url-fetch)
> +              (uri (string-append
> +                    "https://xkbcommon.org/download/libxkbcommon-"
> version
> +                    ".tar.xz"))


> -    (inputs
> -     (list libx11
> -           libxcb
> -           libxml2
> -           wayland
> -           wayland-protocols
> -           xkeyboard-config))
> -    (native-inputs
> -     (list bison doxygen pkg-config python))
> +    (inputs (list libx11
> +                  libxcb
> +                  libxml2
> +                  wayland
> +                  wayland-protocols
> +                  xkeyboard-config))
> +    (native-inputs (list bison doxygen pkg-config python))
Try to avoid mixing cosmetic and non-cosmetic changes.

> +     (list #:configure-flags #~(list (string-append "-Dxkb-config-
> root="
> +                                                    (search-input-
> directory
> +                                                     %build-inputs
> +                                                    
> "share/X11/xkb"))
> +                                     (string-append "-Dx-locale-
> root="
> +                                                    (search-input-
> directory
> +                                                     %build-inputs
> +                                                     
> "share/X11/locale")))))
I am very sure the formatter isn't doing the best job here.

> -    (description "Xkbcommon is a library to handle keyboard
> descriptions,
> +    (description
> +     "Xkbcommon is a library to handle keyboard descriptions,
This change is unnecessary.

Cheers

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

From: Felix Lechner <felix.lechner <at> lease-up.com>
To: 57960 <at> debbugs.gnu.org
Cc: Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at>
Subject: Re: [PATCH core-updates] gnu: libxkbcommon: Switch to gexp;
 adjust style.
Date: Wed, 21 Sep 2022 03:16:29 -0700

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

Hi,

On Wed, Sep 21, 2022 at 2:03 AM Liliana Marie Prikler
<liliana.prikler <at> ist.tugraz.at> wrote:
>
> Try to avoid mixing cosmetic and non-cosmetic changes.

Thanks for the suggestion! I did last time [1] but the core-updates
maintainer seemed to prefer otherwise. [2]

In addition, I was not sure whether to use the formatter from the
'master' branch or from 'core-updates', or whether it made a
difference.

A patch without 'guix style' is attached to this message.

As a side note, my position is that 'guix style' should be applied to
the code base whenever the formatter is changed, and not when
functional changes are being submitted by individual contributors.

[1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=57872#5
[2] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=57872#10

> the formatter isn't doing the best job here.

Do I have a choice? I believe there is a requirement to run 'guix
style' when submitting patches. [3]

[3] https://guix.gnu.org/en/manual/devel/en/html_node/Submitting-Patches.html

> This change is unnecessary.

I respectfully agree. The formatter made that change, too.

Kind regards
Felix Lechner

[0001-gnu-libxkbcommon-Switch-to-gexp-avoid-hardcoding-inp.patch (application/x-patch, attachment)]

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

From: Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at>
To: Felix Lechner <felix.lechner <at> lease-up.com>, 57960 <at> debbugs.gnu.org
Subject: Re: [PATCH core-updates] gnu: libxkbcommon: Switch to gexp; adjust
 style.
Date: Wed, 21 Sep 2022 13:28:57 +0200

Am Mittwoch, dem 21.09.2022 um 03:16 -0700 schrieb Felix Lechner:
> Hi,
> 
> On Wed, Sep 21, 2022 at 2:03 AM Liliana Marie Prikler
> <liliana.prikler <at> ist.tugraz.at> wrote:
> > 
> > Try to avoid mixing cosmetic and non-cosmetic changes.
> 
> Thanks for the suggestion! I did last time [1] but the core-updates
> maintainer seemed to prefer otherwise. [2]
I don't think this was a preference; rather a workaround for some issue
arising in the git workflow (which can happen).

> In addition, I was not sure whether to use the formatter from the
> 'master' branch or from 'core-updates', or whether it made a
> difference.
The answer is "don't change surrounding code", which holds regardless
of the formatter you choose.  I do believe the one on master would be
updated more often, though.

> As a side note, my position is that 'guix style' should be applied to
> the code base whenever the formatter is changed, and not when
> functional changes are being submitted by individual contributors.
I disagree on the basis that the styler is very much not perfect yet
and still causes more issues than not on almost every patch I've so far
reviewed.  There's unfortunately a long way to go before we can mandate
code style. 

> > the formatter isn't doing the best job here.
> 
> Do I have a choice? I believe there is a requirement to run 'guix
> style' when submitting patches. [3]
> 
> [3]
> https://guix.gnu.org/en/manual/devel/en/html_node/Submitting-Patches.html
Most reviewers, myself included, would rather prefer you simply mark
the region/package with Emacs and hit TAB.  However, not all packagers
prefer writing their code in Emacs, hence we tell them to use this
tool, which captures the letter, but sadly not the essence, of the
Scheme coding style.

Cheers
> 

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Felix Lechner <felix.lechner <at> lease-up.com>
Cc: 57960 <at> debbugs.gnu.org,
 Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at>
Subject: Re: bug#57960: [PATCH core-updates] gnu: libxkbcommon: Switch to
 gexp; adjust style.
Date: Sun, 02 Oct 2022 22:43:11 +0200

Hi Felix,

Felix Lechner <felix.lechner <at> lease-up.com> skribis:

> From f07e9206be342f3e9769fe3dc14124b14444f139 Mon Sep 17 00:00:00 2001
> From: Felix Lechner <felix.lechner <at> lease-up.com>
> Date: Tue, 20 Sep 2022 09:47:35 -0700
> Subject: [PATCH] gnu: libxkbcommon: Switch to gexp; avoid hardcoding inputs.
>
> * gnu/packages/xdisorg.scm (libxkbcommon): Switch to gexp.
> [arguments]: Avoid hardcoding inputs and locate them via relative
> paths instead.

Applied, thanks!

As a rule of thumb I agree that formatting changes should not be lumped
together with other changes as that can hinder review.

Regardless, I think a patch like this one should not be blocked for
weeks for such tiny issues.  Insisting on a rigorous process and
conventions is important when it helps us all work together, but it’s a
problem when it hinders cooperation.  As reviewers, we must strike for a
balance that’s reasonable for everyone involved.

Ludo’.

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

From: Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at>
To: Ludovic Courtès <ludo <at> gnu.org>, Felix Lechner
 <felix.lechner <at> lease-up.com>
Cc: 57960 <at> debbugs.gnu.org
Subject: Re: bug#57960: [PATCH core-updates] gnu: libxkbcommon: Switch to
 gexp; adjust style.
Date: Mon, 03 Oct 2022 08:11:43 +0200

Am Sonntag, dem 02.10.2022 um 22:43 +0200 schrieb Ludovic Courtès:
> Regardless, I think a patch like this one should not be blocked for
> weeks for such tiny issues.
Note, that this patch wasn't blocked for this issue, but because I
failed to notice the attachment in which it was already addressed. 
Sometimes, I miss those.

Cheers

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at>
Cc: 57960 <at> debbugs.gnu.org, Felix Lechner <felix.lechner <at> lease-up.com>
Subject: Re: bug#57960: [PATCH core-updates] gnu: libxkbcommon: Switch to
 gexp; adjust style.
Date: Mon, 03 Oct 2022 12:02:45 +0200

Hi Liliana,

Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at> skribis:

> Am Sonntag, dem 02.10.2022 um 22:43 +0200 schrieb Ludovic Courtès:
>> Regardless, I think a patch like this one should not be blocked for
>> weeks for such tiny issues.
> Note, that this patch wasn't blocked for this issue, but because I
> failed to notice the attachment in which it was already addressed. 
> Sometimes, I miss those.

I see, apologies for misrepresenting the situation then.

Thanks,
Ludo’.

Previous Next

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