Re: New review checklist

[Maxime Devos]

Liliana Marie Prikler schreef op vr 01-04-2022 om 19:03 [+0200]:
> Hi Maxime,
> 
> [...]
> > 
> > -- would the commit need to be let-bound here?
> This discussion has already been had elsewhere, but to reiterate, my
> reasoning is that if you can't trust upstream tags to remain valid, you
> need another proof that the VERSION <-> COMMIT equivalence holds. 
> Referring to another authority (as can be done in the case of Minetest
> packages) is fine for me personally, but in the general case (e.g. your
> #2 without further context) I'd say that let-binding the commit leads
> to the least amount of surprises for everyone.

I know there have been some discussions in the past about whether
git-version should be used when a commit is explicitly chosen, whether
tags should be used instead of commits, how high a risk there is that
version->commit can become multi-valued, ‘tricking peer review’ ...

However, my question isn't about any of that.  It is only about the
let-binding itself, in situations where the bound variable is only used
in a single place.  What is the reason for doing

(let ((commit "cabba9e..."))
  (package
    (name "foobar")
    (version "0.1.2")
    (source (origin ...
              ;; this is the only use of the 'commit' variable bound in
              ;; the above 'commit'
              (commit commit)))
    ...))

when it can be simplified to

(package
  (name "foobar")
  (version "0.1.2")
  (source (origin ... (commit "cabba9e..."))))?

I mean, we don't do this for, say, 'name', 'version' and 'uri':

;; these three variables are only used in a single location
(let ((name "foobar")
      (version "0.1.2")
      (uri "https://foo.bar";))
  (package
    (name name)
    (version version)
    (source (origin (uri uri) (commit <some-reference>) [...]))
    ...))

Why would things be different for 'commit' here?  How does putting the
value of 'commit' in a let-form reduce surprises?

Greetings,
Maxime.

signature.asc
Description: This is a digitally signed message part