Questionable "cosmetic changes" commits

[Mark H Weaver <mhw@netris.org>]

[-- Attachment #1: Type: text/plain, Size: 3090 bytes --]

Hello fellow Guix,

In recent months there have been several "cosmetic changes" commits that
I find questionable.  These commits reorder package fields and reindent
code that was already ordered and indented according to our conventions,
apparently in order to match the author's personal preferences.

These commits also sometimes remove useful comments, although this is
not mentioned in the commit logs and not easily noticed in the diffs
amongst the noise of reordering and reindentation.

Here are some recent examples:

https://git.sv.gnu.org/cgit/guix.git/commit/?id=c3264f9e100ad6aefe5216002b68f3bfdcf6be95
https://git.sv.gnu.org/cgit/guix.git/commit/?id=416b1b9f56b514677660b56992cea1c78e00f519
https://git.sv.gnu.org/cgit/guix.git/commit/?id=2394b76bd223f5903e325f1f0e6d6b6fe69d00ed
https://git.sv.gnu.org/cgit/guix.git/commit/?id=d829c348f8a3c4de7e0dedeb4f96913357ae5294
https://git.sv.gnu.org/cgit/guix.git/commit/?id=7c63d0e29f3b33719a2e581d07125a9d03a0ec88
https://git.sv.gnu.org/cgit/guix.git/commit/?id=51e7e72bca9622560cde27db785b2d3e3fe058ae
https://git.sv.gnu.org/cgit/guix.git/commit/?id=0bb718c1b2c8df29ec85a81f002c54061c05ef65
https://git.sv.gnu.org/cgit/guix.git/commit/?id=b168f2ba53b938e1b322c79e5bfa47fcc506b803
https://git.sv.gnu.org/cgit/guix.git/commit/?id=d257697dc1ab4d2a278415d75b057c072f4660ec
https://git.sv.gnu.org/cgit/guix.git/commit/?id=b96961c9d25dd4efeaecf33813f9025282b25869
https://git.sv.gnu.org/cgit/guix.git/commit/?id=98f1951bb93524ed7bd212d884ed17ef21d4653d

I've included below one example for illustration.  The following commit
removes two comments (one about licensing details, and another
explaining why 'libffi' is needed in propagated-inputs), moves the
'home-page' field from its conventional place above the 'synopsis' to
below the 'description' (a common feature among this recent batch of
commits), swaps the order of the 'inputs' and 'native-inputs' fields,
and reindents several fields to be more vertically oriented, as
illustrated by the following change:

___ (native-search-paths
____ (list (search-path-specification
___________ (variable "GI_TYPELIB_PATH")
___________ (files '("lib/girepository-1.0")))))

was changed to:

___ (native-search-paths
____ (list
_____ (search-path-specification
______ (variable "GI_TYPELIB_PATH")
______ (files '("lib/girepository-1.0")))))

I think that commits like this are best avoided for several reasons.
Most importantly, they make merges between branches more error prone.

We all have our own personal preferences of how best to indent scheme
code, but if more of us adopted the habit of needlessly reordering
fields and reindenting code of every package we touch, as one of us
seems to have done, it could get out of hand quickly.  For example, my
personal preference would be to reverse the indentation change of the
'native-search-paths' field shown above, and to move the 'home-page'
field back above the 'synopsis' to match our usual conventions.  Should
I change those things back the next time I update that package?

What do other people think?

      Regards,
        Mark


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: [PATCH] gnu: gobject-introspection: Make some cosmetic changes --]
[-- Type: text/x-patch, Size: 3750 bytes --]

From c3264f9e100ad6aefe5216002b68f3bfdcf6be95 Mon Sep 17 00:00:00 2001
From: Raghav Gururajan <raghavgururajan@disroot.org>
Date: Thu, 24 Sep 2020 08:57:59 -0400
Subject: [PATCH] gnu: gobject-introspection: Make some cosmetic changes.

* gnu/packages/glib.scm (gobject-introspection): Make some cosmetic changes.

Signed-off-by: Danny Milosavljevic <dannym@scratchpost.org>
---
 gnu/packages/glib.scm | 45 ++++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/gnu/packages/glib.scm b/gnu/packages/glib.scm
index 43523e516d..a4fa6faefb 100644
--- a/gnu/packages/glib.scm
+++ b/gnu/packages/glib.scm
@@ -429,17 +429,20 @@ dynamic loading, and an object system.")
   (package
     (name "gobject-introspection")
     (version "1.62.0")
-    (source (origin
-             (method url-fetch)
-             (uri (string-append "mirror://gnome/sources/"
-                   "gobject-introspection/" (version-major+minor version)
-                   "/gobject-introspection-" version ".tar.xz"))
-             (sha256
-              (base32 "18lhglg9v6y83lhqzyifc1z0wrlawzrhzzxx0a3h1g7xaz97xvmi"))
-             (patches (search-patches
-                       "gobject-introspection-cc.patch"
-                       "gobject-introspection-girepository.patch"
-                       "gobject-introspection-absolute-shlib-path.patch"))))
+    (source
+     (origin
+       (method url-fetch)
+       (uri
+        (string-append "mirror://gnome/sources/"
+                       "gobject-introspection/" (version-major+minor version)
+                       "/gobject-introspection-" version ".tar.xz"))
+       (sha256
+        (base32 "18lhglg9v6y83lhqzyifc1z0wrlawzrhzzxx0a3h1g7xaz97xvmi"))
+       (patches
+        (search-patches
+         "gobject-introspection-cc.patch"
+         "gobject-introspection-girepository.patch"
+         "gobject-introspection-absolute-shlib-path.patch"))))
     (build-system meson-build-system)
     (arguments
      `(#:phases
@@ -450,25 +453,23 @@ dynamic loading, and an object system.")
                (("#!@PYTHON_CMD@")
                 (string-append "#!" (which "python3"))))
              #t)))))
+    (native-inputs
+     `(("glib" ,glib "bin")
+       ("pkg-config" ,pkg-config)))
     (inputs
      `(("bison" ,bison)
        ("flex" ,flex)
        ("glib" ,glib)
        ("python" ,python-wrapper)
        ("zlib" ,zlib)))
-    (native-inputs
-     `(("glib" ,glib "bin")
-       ("pkg-config" ,pkg-config)))
     (propagated-inputs
-     `(;; In practice, GIR users will need libffi when using
-       ;; gobject-introspection.
-       ("libffi" ,libffi)))
+     `(("libffi" ,libffi)))
     (native-search-paths
-     (list (search-path-specification
-            (variable "GI_TYPELIB_PATH")
-            (files '("lib/girepository-1.0")))))
+     (list
+      (search-path-specification
+       (variable "GI_TYPELIB_PATH")
+       (files '("lib/girepository-1.0")))))
     (search-paths native-search-paths)
-    (home-page "https://wiki.gnome.org/GObjectIntrospection")
     (synopsis "Generate interface introspection data for GObject libraries")
     (description
      "GObject introspection is a middleware layer between C libraries (using
@@ -476,7 +477,7 @@ GObject) and language bindings.  The C library can be scanned at compile time
 and generate a metadata file, in addition to the actual native C library.  Then
 at runtime, language bindings can read this metadata and automatically provide
 bindings to call into the C library.")
-    ; Some bits are distributed under the LGPL2+, others under the GPL2+
+    (home-page "https://wiki.gnome.org/GObjectIntrospection")
     (license license:gpl2+)))
 
 (define intltool
-- 
2.29.2