Subject: Re: bug#50072: [PATCH WIP 0/4] Add upstream updater for git-fetch origins.

From: Ludovic Courtès <[email-masked]>
Date: Mon, 06 Sep 2021 12:27:42 +0200

Hi Sarah,

I like this patch series.  :-)

Sarah Morgensen <[email-masked]> skribis:

> * guix/git-download.scm (checkout-to-store): New procedure.
> * guix/upstream.scm (guess-version-transform)
> (package-update/git-fetch): New procedures.
> (%method-updates): Add GIT-FETCH mapping.

This LGTM.

Nitpick:

> +(define* (checkout-to-store store ref #:key (log (current-error-port)))
> +  "Checkout REF to STORE.  Write progress reports to LOG.  RECURSIVE? has the
> +same effect as the same-named parameter of 'latest-repository-commit'."
> +  ;; XXX: (guix git) does not use shallow clones, so this will be slow
> +  ;; for long-running repositories.
> +  (match-record ref <git-reference>

[...]

> +  ;; Only use the first element of URLS.
> +  (match-record source <upstream-source>
> +    (version urls)

I’d use the record acceesors in this cases rather than ‘match-record’;
currently ‘match-record’ is not super efficient and I find it slightly
less readable when you’re just accessing a couple of fields.

Thanks,
Ludo’.