[bug#31277] [PATCH 2/3] services: cgit: Improve handling of extra-options.

[Clément Lassieur]

Christopher Baines <address@hidden> writes:

> * gnu/services/cgit.scm (serialize-cgit-configuration): Add the extra options,
> one per line, before the scan-path, as this makes it possible to use the
> extra-options to affect the global behaviour for repositories.
> ---
>  gnu/services/cgit.scm | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/services/cgit.scm b/gnu/services/cgit.scm
> index 8ef12cd5a..bacd6ca16 100644
> --- a/gnu/services/cgit.scm
> +++ b/gnu/services/cgit.scm
> @@ -642,13 +642,19 @@ for cgit to allow access to that repository.")
>      (not (memq (configuration-field-name field)
>                 '(project-list
>                   repository-directory
> -                 repositories))))
> +                 repositories
> +                 extra-options))))

                      ---^
Very small nitpick: could you put extra-options between project-list and
repository-directory there?  So that the order is the same with what
comes below.

>    #~(string-append
>       #$(let ((rest (filter rest? cgit-configuration-fields)))
>           (serialize-configuration config rest))
>       #$(serialize-project-list
>          'project-list
>          (cgit-configuration-project-list config))
> +     #$(string-join
> +        (append
> +         (cgit-configuration-extra-options config)
> +         '(""))
> +        "\n")

Could you put the serialization code in a dedicated function?
(e.g. SERIALIZE-EXTRA-OPTION)  So that SERIALIZE-CGIT-CONFIGURATION is
only responsible for the field ordering.

Also, I don't understand why you append '("").  If it's just to make
cgitrc look prettier, I'm not sure it's a good idea.  But maybe I missed
something?

Otherwise LGTM :-)

Clément