Regarding the vertical alignment in the record definitions

[Tomas Volf <~@wolfsden.cz>]

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


Hello Guix,

I would like to bring up for debate the convention the project has of
vertically aligning the record definitions.  While I agree it lead to
visually pleasing code, I also leads to significantly bloated diffs.

While some amount of noise in diffs is expected in any lisp language
(the trailing `)'), the vertical aligning makes it worse.  It (in my
opinion) complicates both reviewing and re-basing (due to merge
conflicts).

For example, take this patch I sent today.  It adds *a single field* to
the record.  However the patch (that someone needs to review and make
sure is correct) looks like this:

--8<---------------cut here---------------start------------->8---
@@ -1246,44 +1247,49 @@ (define-deprecated (agetty-service config)
 (define-record-type* <mingetty-configuration>
   mingetty-configuration make-mingetty-configuration
   mingetty-configuration?
-  (mingetty         mingetty-configuration-mingetty ;file-like
-                    (default mingetty))
-  (tty              mingetty-configuration-tty)       ;string
-  (auto-login       mingetty-configuration-auto-login ;string | #f
-                    (default #f))
-  (login-program    mingetty-configuration-login-program ;gexp
-                    (default #f))
-  (login-pause?     mingetty-configuration-login-pause? ;Boolean
-                    (default #f))
-  (clear-on-logout? mingetty-configuration-clear-on-logout? ;Boolean
-                    (default #t))
-  (delay            mingetty-configuration-delay ;Integer | #f
-                    (default #f))
-  (print-issue      mingetty-configuration-print-issue ;Boolean | Symbol
-                    (default #t))
-  (print-hostname   mingetty-configuration-print-hostname ;Boolean | Symbol
-                    (default #t))
-  (nice             mingetty-configuration-nice ;Integer | #f
-                    (default #f))
-  (chdir            mingetty-configuration-chdir ;String | #f
-                    (default #f))
-  (chroot           mingetty-configuration-chroot ;String | #f
-                    (default #f)))
+  (mingetty             mingetty-configuration-mingetty ;file-like
+                        (default mingetty))
+  (tty                  mingetty-configuration-tty)       ;string
+  (auto-login           mingetty-configuration-auto-login ;string | #f
+                        (default #f))
+  (login-program        mingetty-configuration-login-program ;gexp
+                        (default #f))
+  (login-pause?         mingetty-configuration-login-pause? ;Boolean
+                        (default #f))
+  (clear-on-logout?     mingetty-configuration-clear-on-logout? ;Boolean
+                        (default #t))
+  (delay                mingetty-configuration-delay ;Integer | #f
+                        (default #f))
+  (print-issue          mingetty-configuration-print-issue ;Boolean | Symbol
+                        (default #t))
+  (print-hostname       mingetty-configuration-print-hostname ;Boolean | Symbol
+                        (default #t))
+  (nice                 mingetty-configuration-nice ;Integer | #f
+                        (default #f))
+  (chdir                mingetty-configuration-chdir ;String | #f
+                        (default #f))
+  (chroot               mingetty-configuration-chroot ;String | #f
+                        (default #f))
+  (shepherd-requirement mingetty-configuration-shepherd-requirement
+                        ;; Since the login prompt shows the host name, wait
+                        ;; for the 'host-name' service to be done.  Also wait
+                        ;; for udev essentially so that the tty text is not
+                        ;; lost in the middle of kernel messages (XXX).
+                        (default '( user-processes host-name udev
+                                    virtual-terminal))))
--8<---------------cut here---------------end--------------->8---

But it could have look like this:

--8<---------------cut here---------------start------------->8---
@@ -1268,23 +1269,27 @@ (define-record-type* <mingetty-configuration>
   (working-directory mingetty-configuration-working-directory ;String | #f
                      (default #f))
   (root-directory    mingetty-configuration-root-directory ;String | #f
-                     (default #f)))
+                     (default #f))
+  (shepherd-requirement mingetty-configuration-shepherd-requirement
+                        ;; Since the login prompt shows the host name, wait
+                        ;; for the 'host-name' service to be done.  Also wait
+                        ;; for udev essentially so that the tty text is not
+                        ;; lost in the middle of kernel messages (XXX).
+                        (default '( user-processes host-name udev
+                                    virtual-terminal))))
--8<---------------cut here---------------end--------------->8---

I would like to hear what people who review lot of patches think about
this.  Does it make your life worse?  Does it not matter?  Or does it
even help you in some way?

Thank you for your time and have a nice day,
Tomas

-- 
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]