[bug#28283] Status: [PATCH 1/1] gnu: services: version-control: Add cgit.

[ludo@gnu.org (Ludovic Courtès)]

Hi Oleg,

Oleg Pykhalov <go.wigust@gmail.com> skribis:

> From 65fa66cd761f3a9f8c6e84f8b5f7d8c643fe9731 Mon Sep 17 00:00:00 2001
> From: Oleg Pykhalov <go.wigust@gmail.com>
> Date: Tue, 29 Aug 2017 23:40:05 +0300
> Subject: [PATCH 1/1] gnu: services: version-control: Add cgit.
>
> * gnu/services/version-control.scm
> (<cgit-configuration-file>, <cgit-configuration>): New record types.
> (cgit-configuration-robots-string, cgit-activation,
> cgit-configuration-nginx-config): New procedures.
> (%cgit-configuration-nginx, cgit-service-type): New variables.
> * doc/guix.texi (Version Control): Document the cgit service.
> * gnu/services/web.scm (<nginx-server-configuration>): Add
> nginx-server-configuration-try-files.
> (emit-nginx-server-config): Add this.

That looks nice and useful!

Minor issues:

> +@subsubheading cgit service

Please capitalize titles: “Cgit Service”

> +@uref{https://git.zx2c4.com/cgit/, cgit} is a hyperfast web
> +frontend for git repositories written in C.

“Git”, not “git”.  Also, not hypersuperlatives please.  :-)

So what about this:

  @uref{https://git.zx2c4.com/cgit/, cgit} is a Web interface to Git
  repositories, written in C.  It allows users to look at Git
  repository contents and history through their Web browsers.

> +The following example will configure the service with default values.
> +By default, cgit can be accessed on port 80 (@code{http://localhost:80}).
> +
> +@example
> +(service nginx-service-type)
> +(service fcgiwrap-service-type)
> +(service cgit-service-type)
> +@end example

Please add a sentence above or below the example like “This example
shows …”.

> +@table @asis
> +@item @code{css} (default: @code{"/share/cgit/cgit.css"})
> +Url which specifies the css document to include in all cgit pages.
> +
> +@item @code{logo} (default: @code{"/share/cgit/cgit.png"})
> +Url which specifies the source of an image which will be used as a logo
> +on all cgit pages.
> +
> +@item @code{virtual-root} (default: @code{"/"})
> +Url which, if specified, will be used as root for all cgit links.

s/Url/URL/

> +@item @code{scan-path} (default: @code{"/srv/git"})
> +A path which will be scanned for repositories.

Rather: “Name of the directory to scan for repositories.”

Can you also rename ‘scan-path’ to ‘repository-directory’ or something
like that?  (Not “path.”)

> +@item @code{robots} (default: @code{(list "noindex" "nofollow")})
> +Text used as content for the "robots" meta-tag.

Write ``robots'' (literally) so that Texinfo typesets things correctly.

> --- a/gnu/services/web.scm
> +++ b/gnu/services/web.scm
> @@ -99,6 +99,8 @@
>                         (default '()))
>    (index               nginx-server-configuration-index
>                         (default (list "index.html")))
> +  (try-files           nginx-server-configuration-try-files
> +                       (default #f))
>    (ssl-certificate     nginx-server-configuration-ssl-certificate
>                         (default "/etc/nginx/cert.pem"))
>    (ssl-certificate-key nginx-server-configuration-ssl-certificate-key
> @@ -179,6 +181,7 @@ of index files."
>           (nginx-server-configuration-ssl-certificate-key server))
>          (root (nginx-server-configuration-root server))
>          (index (nginx-server-configuration-index server))
> +        (try-files (nginx-server-configuration-try-files server))
>          (server-tokens? (nginx-server-configuration-server-tokens? server))
>          (locations (nginx-server-configuration-locations server)))
>      (define-syntax-parameter <> (syntax-rules ()))
> @@ -207,6 +210,7 @@ of index files."
>       (and/l ssl-certificate-key "      ssl_certificate_key " <> ";\n")
>       "      root " root ";\n"
>       "      index " (config-index-strings index) ";\n"
> +     "      try_files " (config-index-strings try-files) ";\n"
>       "      server_tokens " (if server-tokens? "on" "off") ";\n"
>       "\n"
>       (map emit-nginx-location-config locations)

Could you submit these nginx changes separately for discussion?  We’ll
also need to document them.  Maybe Cc Chris Baines and other nginx
people.

Thanks!

Ludo’.