diff mbox series

[bug#57960,core-updates] gnu: libxkbcommon: Switch to gexp; adjust style.

Message ID CAFHYt55gav8sDkg=m-S57TKwNaBVRumnwsj0XXt1KMT5iLPNtA@mail.gmail.com
State New
Headers show
Series [bug#57960,core-updates] gnu: libxkbcommon: Switch to gexp; adjust style. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git-branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

Felix Lechner Sept. 20, 2022, 7:23 p.m. UTC
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

Comments

Liliana Marie Prikler Sept. 21, 2022, 9:03 a.m. UTC | #1
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
Felix Lechner Sept. 21, 2022, 10:16 a.m. UTC | #2
Hi,

On Wed, Sep 21, 2022 at 2:03 AM Liliana Marie Prikler
<liliana.prikler@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
Liliana Marie Prikler Sept. 21, 2022, 11:28 a.m. UTC | #3
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@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
>
diff mbox series

Patch

From 81ffbadce5c31f447fce7970908a51d483753b14 Mon Sep 17 00:00:00 2001
From: Felix Lechner <felix.lechner@lease-up.com>
Date: Tue, 20 Sep 2022 09:47:35 -0700
Subject: [PATCH] gnu: libxkbcommon: Switch to gexp; adjust style.

* gnu/packages/xdisorg.scm (libxkbcommon): Switch to gexp; adjust
style throughout.
[arguments]: Avoid hardcoding inputs; locate inputs via relative
paths, i.e. the modern way.
---
 gnu/packages/xdisorg.scm | 47 ++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/gnu/packages/xdisorg.scm b/gnu/packages/xdisorg.scm
index 02e04ccf88..aeacdf8743 100644
--- a/gnu/packages/xdisorg.scm
+++ b/gnu/packages/xdisorg.scm
@@ -488,33 +488,34 @@  (define-public libxkbcommon
     (name "libxkbcommon")
     (version "1.3.0")
     (source (origin
-             (method url-fetch)
-             (uri (string-append "https://xkbcommon.org/download/libxkbcommon-"
-                                 version ".tar.xz"))
-             (sha256
-              (base32
-               "0ysynzzgzd9jdrh1321r4bgw8wd5zljrlyn5y1a31g39xacf02bv"))))
+              (method url-fetch)
+              (uri (string-append
+                    "https://xkbcommon.org/download/libxkbcommon-" version
+                    ".tar.xz"))
+              (sha256
+               (base32
+                "0ysynzzgzd9jdrh1321r4bgw8wd5zljrlyn5y1a31g39xacf02bv"))))
     (build-system meson-build-system)
-    (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))
     (arguments
-     `(#:configure-flags
-       (list (string-append "-Dxkb-config-root="
-                            (assoc-ref %build-inputs "xkeyboard-config")
-                            "/share/X11/xkb")
-             (string-append "-Dx-locale-root="
-                            (assoc-ref %build-inputs "libx11")
-                            "/share/X11/locale"))))
+     (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")))))
     (home-page "https://xkbcommon.org/")
     (synopsis "Library to handle keyboard descriptions")
-    (description "Xkbcommon is a library to handle keyboard descriptions,
+    (description
+     "Xkbcommon is a library to handle keyboard descriptions,
 including loading them from disk, parsing them and handling their
 state.  It is mainly meant for client toolkits, window systems, and other
 system applications; currently that includes Wayland, kmscon, GTK+, Qt,

base-commit: 28918b4ff8d2c297d1f1fb84eebdca9fc2dc9303
-- 
2.37.3