all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [RFC] Cosmetic changes to define-configuration usage
@ 2023-03-24 12:33 Bruno Victal
  2023-03-28 16:20 ` Katherine Cox-Buday
  2023-03-31 14:46 ` Maxim Cournoyer
  0 siblings, 2 replies; 5+ messages in thread
From: Bruno Victal @ 2023-03-24 12:33 UTC (permalink / raw)
  To: guix-devel; +Cc: mirai, Maxim Cournoyer

Forwarded from: <https://logs.guix.gnu.org/guix/2023-03-22.log#165538> & [1]

--8<---------------cut here---------------start------------->8---
<mirai>apteryx: IMO the spacing between the fields should have been kept
<mirai>it makes things easier to read
<mirai>it's a nightmare if the records grow very large
<apteryx>mirai: I was on the fence about it, but keeping the fields together in the same record appears to be the more conventional style in the code base
<apteryx>(together as in without blank lines in-between)
<mirai>apteryx: I'm planning to gradually shift the define-configurations to have a space between fields
<apteryx>mirai: it should be discussed first to guix-devel :-)
--8<---------------cut here---------------end--------------->8---


I'd like to propose for field specifications in define-configuration to be separated with a
blank line between them. Reason for this is that it makes it much easier on the eyes
to read, rather than being presented with a dense hunk of text to sift through.

I tend to always insert these blank lines when making changes in these parts to aid reading,
even if they weren't originally present and were not planned to be committed. I'd be happy if
I could simply keep the line separations and avoid the tedious insert-erase ritual.

I think I'm not alone in this opinion, consider the following snippets:


With a line separating each field:   (gnu/services/mcron.scm)
--8<---------------cut here---------------start------------->8---
(define-configuration/no-serialization mcron-configuration
  (mcron
   (file-like mcron)
   "The mcron package to use.")

  (jobs
   (list-of-gexps '())
   "This is a list of gexps (@pxref{G-Expressions}), where each gexp
corresponds to an mcron job specification (@pxref{Syntax, mcron job
specifications,, mcron, GNU@tie{}mcron}).")

  (log?
   (boolean #t)
   "Log messages to standard output.")

  (log-file
   (string "/var/log/mcron.log")
   "Log file location.")

  (log-format
   (string "~1@*~a ~a: ~a~%")
   "@code{(ice-9 format)} format string for log messages.  The default value
produces messages like @samp{@var{pid} @var{name}: @var{message}}
(@pxref{Invoking mcron, Invoking,, mcron, GNU@tie{}mcron}).
Each message is also prefixed by a timestamp by GNU Shepherd.")

  (date-format
   maybe-string
   "@code{(srfi srfi-19)} format string for date."))
--8<---------------cut here---------------end--------------->8---


Lines collapsed:   (gnu/services/linux.scm)
--8<---------------cut here---------------start------------->8---
(define-configuration fstrim-configuration
  (package
    (file-like util-linux)
    "The package providing the @command{fstrim} command."
    empty-serializer)
  (schedule
   (mcron-time "0 0 * * 0")
   "Schedule for launching @command{fstrim}.  This can be a procedure, a list
or a string.  For additional information, see @ref{Guile Syntax,,
Job specification, mcron, the mcron manual}.  By default this is set to run
weekly on Sunday at 00:00."
   empty-serializer)
  ;; The following are fstrim-related options.
  (listed-in
   (maybe-list-of-strings '("/etc/fstab" "/proc/self/mountinfo"))
   ;; Note: documentation sourced from the fstrim manpage.
   "List of files in fstab or kernel mountinfo format.  All missing or
empty files are silently ignored.  The evaluation of the list @emph{stops}
after the first non-empty file.  File systems with @code{X-fstrim.notrim} mount
option in fstab are skipped.")
  (verbose?
   (boolean #t)
   "Verbose execution.")
  (quiet-unsupported?
   (boolean #t)
   "Suppress error messages if trim operation (ioctl) is unsupported.")
  (extra-arguments
   maybe-list-of-strings
   "Extra options to append to @command{fstrim} (run @samp{man fstrim} for
more information)."
   (lambda (_ value)
     (if (maybe-value-set? value)
         value '())))
--8<---------------cut here---------------end--------------->8---


IMO, the top snippet looks much less overwhelming than the bottom one.


## “Implementation” plan

This cosmetic change can be done gradually or with a commit that performs the restyling in bulk.


[1]: <https://issues.guix.gnu.org/61964>


Cheers,
Bruno


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC] Cosmetic changes to define-configuration usage
  2023-03-24 12:33 [RFC] Cosmetic changes to define-configuration usage Bruno Victal
@ 2023-03-28 16:20 ` Katherine Cox-Buday
  2023-03-31 14:46 ` Maxim Cournoyer
  1 sibling, 0 replies; 5+ messages in thread
From: Katherine Cox-Buday @ 2023-03-28 16:20 UTC (permalink / raw)
  To: guix-devel

On 3/24/23 6:33 AM, Bruno Victal wrote:

> I'd like to propose for field specifications in define-configuration to be separated with a
> blank line between them. Reason for this is that it makes it much easier on the eyes
> to read, rather than being presented with a dense hunk of text to sift through.
> 
> I tend to always insert these blank lines when making changes in these parts to aid reading,
> even if they weren't originally present and were not planned to be committed. I'd be happy if
> I could simply keep the line separations and avoid the tedious insert-erase ritual.
> 
> I think I'm not alone in this opinion, consider the following snippets:

It's funny how sometimes this works out. I was just hacking on a 
service's configuration and had the same thought. I had spaced 
everything out so that it wasn't as overwhelming, and with a sigh 
grouped it all back together because that's how all existing services work.

I would definitely appreciate this change in the style standard.

--
Katherine



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC] Cosmetic changes to define-configuration usage
  2023-03-24 12:33 [RFC] Cosmetic changes to define-configuration usage Bruno Victal
  2023-03-28 16:20 ` Katherine Cox-Buday
@ 2023-03-31 14:46 ` Maxim Cournoyer
  2023-04-03 14:13   ` Bruno Victal
  1 sibling, 1 reply; 5+ messages in thread
From: Maxim Cournoyer @ 2023-03-31 14:46 UTC (permalink / raw)
  To: Bruno Victal; +Cc: guix-devel

Hello,

Bruno Victal <mirai@makinata.eu> writes:

> Forwarded from: <https://logs.guix.gnu.org/guix/2023-03-22.log#165538> & [1]
>
> <mirai>apteryx: IMO the spacing between the fields should have been kept
> <mirai>it makes things easier to read
> <mirai>it's a nightmare if the records grow very large
> <apteryx>mirai: I was on the fence about it, but keeping the fields together in the same record appears to be the more conventional style in the code base
> <apteryx>(together as in without blank lines in-between)
> <mirai>apteryx: I'm planning to gradually shift the define-configurations to have a space between fields
> <apteryx>mirai: it should be discussed first to guix-devel :-)
>
>
>
> I'd like to propose for field specifications in define-configuration to be separated with a
> blank line between them. Reason for this is that it makes it much easier on the eyes
> to read, rather than being presented with a dense hunk of text to sift through.
>
> I tend to always insert these blank lines when making changes in these parts to aid reading,
> even if they weren't originally present and were not planned to be committed. I'd be happy if
> I could simply keep the line separations and avoid the tedious insert-erase ritual.
>
> I think I'm not alone in this opinion, consider the following snippets:
>
>
> With a line separating each field:   (gnu/services/mcron.scm)
>
> (define-configuration/no-serialization mcron-configuration
>   (mcron
>    (file-like mcron)
>    "The mcron package to use.")
>
>   (jobs
>    (list-of-gexps '())
>    "This is a list of gexps (@pxref{G-Expressions}), where each gexp
> corresponds to an mcron job specification (@pxref{Syntax, mcron job
> specifications,, mcron, GNU@tie{}mcron}).")
>
>   (log?
>    (boolean #t)
>    "Log messages to standard output.")
>
>   (log-file
>    (string "/var/log/mcron.log")
>    "Log file location.")
>
>   (log-format
>    (string "~1@*~a ~a: ~a~%")
>    "@code{(ice-9 format)} format string for log messages.  The default value
> produces messages like @samp{@var{pid} @var{name}: @var{message}}
> (@pxref{Invoking mcron, Invoking,, mcron, GNU@tie{}mcron}).
> Each message is also prefixed by a timestamp by GNU Shepherd.")
>
>   (date-format
>    maybe-string
>    "@code{(srfi srfi-19)} format string for date."))
>
>
>
> Lines collapsed:   (gnu/services/linux.scm)
>
> (define-configuration fstrim-configuration
>   (package
>     (file-like util-linux)
>     "The package providing the @command{fstrim} command."
>     empty-serializer)
>   (schedule
>    (mcron-time "0 0 * * 0")
>    "Schedule for launching @command{fstrim}.  This can be a procedure, a list
> or a string.  For additional information, see @ref{Guile Syntax,,
> Job specification, mcron, the mcron manual}.  By default this is set to run
> weekly on Sunday at 00:00."
>    empty-serializer)
>   ;; The following are fstrim-related options.
>   (listed-in
>    (maybe-list-of-strings '("/etc/fstab" "/proc/self/mountinfo"))
>    ;; Note: documentation sourced from the fstrim manpage.
>    "List of files in fstab or kernel mountinfo format.  All missing or
> empty files are silently ignored.  The evaluation of the list @emph{stops}
> after the first non-empty file.  File systems with @code{X-fstrim.notrim} mount
> option in fstab are skipped.")
>   (verbose?
>    (boolean #t)
>    "Verbose execution.")
>   (quiet-unsupported?
>    (boolean #t)
>    "Suppress error messages if trim operation (ioctl) is unsupported.")
>   (extra-arguments
>    maybe-list-of-strings
>    "Extra options to append to @command{fstrim} (run @samp{man fstrim} for
> more information)."
>    (lambda (_ value)
>      (if (maybe-value-set? value)
>          value '())))

I have some apprehension that if we start adding white space between the
fields here, we'll soon have people adding white space to many other
places (for consistency or other reasons), which I wouldn't welcome (I
value compactness, and since in Scheme a single newline is used to
delimit things at the top level, too much of white space can make things
less readable in my opinion).

-- 
Thanks,
Maxim


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC] Cosmetic changes to define-configuration usage
  2023-03-31 14:46 ` Maxim Cournoyer
@ 2023-04-03 14:13   ` Bruno Victal
  2023-04-08 19:55     ` Maxim Cournoyer
  0 siblings, 1 reply; 5+ messages in thread
From: Bruno Victal @ 2023-04-03 14:13 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: guix-devel, cox.katherine.e

On 2023-03-31 15:46, Maxim Cournoyer wrote:> I have some apprehension that if we start adding white space between the
> fields here, we'll soon have people adding white space to many other
> places (for consistency or other reasons), which I wouldn't welcome (I
> value compactness, and since in Scheme a single newline is used to
> delimit things at the top level, too much of white space can make things
> less readable in my opinion).

I don't think it needs to be an all-or-nothing situation,
the spacing rules can be always applied selectively “when it makes sense”.

I think spaces between fields is consistent with the general way of things,
for instance, throughout Guix, sections that are only scheme code often do
have some spaces here and there that were added without any adherence to some
rigid criteria but the programmer found it to be an adequate point to 
partition the logic.

The same reasoning applies here, the logic partitioning is done per field instead.
Objectively, there's also a small quantitative difference that's not commonly
present in the rest of the codebase. define-configuration handles both code and documentation,
or putting it another way, it intersperses code and (rather long) strings. The result
is that it's particularly information-dense compared to any other part of the guix codebase.


My 2¢!


Cheers,
Bruno


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC] Cosmetic changes to define-configuration usage
  2023-04-03 14:13   ` Bruno Victal
@ 2023-04-08 19:55     ` Maxim Cournoyer
  0 siblings, 0 replies; 5+ messages in thread
From: Maxim Cournoyer @ 2023-04-08 19:55 UTC (permalink / raw)
  To: Bruno Victal; +Cc: guix-devel, cox.katherine.e

Hi Bruno,

Bruno Victal <mirai@makinata.eu> writes:

> On 2023-03-31 15:46, Maxim Cournoyer wrote:> I have some apprehension
> that if we start adding white space between the
>> fields here, we'll soon have people adding white space to many other
>> places (for consistency or other reasons), which I wouldn't welcome (I
>> value compactness, and since in Scheme a single newline is used to
>> delimit things at the top level, too much of white space can make things
>> less readable in my opinion).
>
> I don't think it needs to be an all-or-nothing situation,
> the spacing rules can be always applied selectively “when it makes sense”.
>
> I think spaces between fields is consistent with the general way of things,
> for instance, throughout Guix, sections that are only scheme code often do
> have some spaces here and there that were added without any adherence to some
> rigid criteria but the programmer found it to be an adequate point to 
> partition the logic.
>
> The same reasoning applies here, the logic partitioning is done per field instead.
> Objectively, there's also a small quantitative difference that's not commonly
> present in the rest of the codebase. define-configuration handles both code and documentation,
> or putting it another way, it intersperses code and (rather long) strings. The result
> is that it's particularly information-dense compared to any other part of the guix codebase.

You make convincing arguments.  If it's understood that it makes sense
in this specific case and not to be taken as "let's add white space to
all the records in the Guix code base", I don't oppose the change, if
people find it makes a difference for the better in terms of readability.

Are there people who would like to block the change?  Otherwise, I'll
contribute it as-is in a few days.

-- 
Thanks,
Maxim


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-04-08 19:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-24 12:33 [RFC] Cosmetic changes to define-configuration usage Bruno Victal
2023-03-28 16:20 ` Katherine Cox-Buday
2023-03-31 14:46 ` Maxim Cournoyer
2023-04-03 14:13   ` Bruno Victal
2023-04-08 19:55     ` Maxim Cournoyer

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.