On Tue, Dec 03, 2024 at 01:34:29AM +0100, Tomas Volf wrote: > > 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
I agree it makes for a more bloated diff, but it does make it easier to
read when the indentation doesn't change between each field. What if it
were formatted like this? No gratuitous white space changes for
formatting and more room for code and comments since it's not smushed on
the right side of the screen.
--8<---------------cut here---------------start------------->8---
@@ -1268,23 +1269,28 @@ (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---
--
Efraim Flashner <efr...@flashner.co.il> אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted
signature.asc
Description: PGP signature
