New review checklist

New review checklist

[Liliana Marie Prikler]

Dear reviewer,

in the sequel find the new review checklist, effective immediately. 
Failure to apply it will result in the confiscation of your machine for
the purpose of making it usable for continuous integration.

Happy April Fools

-----

So you want to package a
  [ ] C  [ ] C++  [ ] C#  [ ] Common Lisp  [ ] Emacs Lisp  [ ] Fortran
  [ ] Guile  [ ] Haskell  [ ] Java  [ ] Javascript  [ ] Julia  [ ] Nim
  [ ] OCaml  [ ] Python  [ ] R  [ ] Rust  [ ] V  [ ] Zig  [ ] ________
application/library/________.
It won't be added to Guix.  Here's why it won't.

You appear to believe that
[ ] linter warnings can easily be ignored
[ ] `make check' does not need to succeed
[ ] nobody will ever want to build your package on
   [ ] x86_64  [ ] i686  [ ] aarch64  [ ] armhf  [ ] mips____
   [ ] powerpc____  [ ] riscv__  [ ] ______-mingw32  [ ] the Hurd
[ ] commit hashes make for good version numbers
[ ] hard-coding the commit field is a good idea
[ ] using trivial-build system is a good idea
[ ] we hard-code
   [ ] invocations of command line tools
   [ ] shared libraries
   [ ] _________
   for fun
[ ] updating ______ to add your package does not cause a world rebuild
[ ] committers have nothing better to do than trailing a branch that
    receives _____ commits per day.

Sadly your patch has/lacks
[ ] copyright headers
[ ] changes in other parts of the file
[ ] indentation
[ ] speling misstakes
[ ] new-style inputs
[ ] propagated inputs
[ ] a useful synopsis
[ ] a meaningful description
[ ] a valid home-page
[ ] correct licensing information
[ ] significant improvements over the three other patches adding this  
    package, which themselves are stuck in review hell

The following technophilosophical objections also apply:
[ ] the GNU FSDG prohibit _____________________________
[ ] your package bundles a meaningless copy of
   [ ] ffmpeg
   [ ] v8
   [ ] font-awesome
   [ ] bundler
   [ ] rustc
   [ ] ________
[ ] your package bootstraps itself using a sparse autoencoder trained
    on /dev/urandom
[ ] your package is not reproducible thanks to
   [ ] embedded timestamps
   [ ] CPU feature detection during configure/compile time
   [ ] a flaky test suite
   [ ] an evil hack to call rand() inside a constexpr context
[ ] Guix should not have to carry every fork of suckmore tools

In conclusion, this is what I think of you:
[ ] Your patch looks good, but I'm not going to push it.
[ ] Your patch would need some work, and I'm not going to invest that
    time on your behalf.
[ ] Your patch is bad and you should feel bad for submitting it.
[ ] Maintaining this package in your own channel is an adequate
    punishment for writing it.

Re: New review checklist

[Maxime Devos]

[-- Attachment #1: Type: text/plain, Size: 1048 bytes --]

Liliana Marie Prikler schreef op vr 01-04-2022 om 06:14 [+0200]:
> It won't be added to Guix.  Here's why it won't.
> You appear to believe that
> [...]
> [ ] hard-coding the commit field is a good idea

Does the following count:

(define-public foo
  (package
    (name "foo")
    (version "1.0.0")
    (source
      (origin
        [...]
        (file-name (git-file-name name version))
        ;; Upstream does not tag versions, see
        ;; <https://foo.bar/versions> for which commit
        ;; corresponds to which version.
        ;;
        ;; (alternatively)
        ;;
        ;; Upstream deletes old tags every N months,
        ;; so explicitly write the commit here.;
        ;;
        ;; (alternatively)
        ;;
        ;; There is consensus that the benefits of explicit commits
        ;; outweigh the downsides, see
        ;; <https://actually.I.dont.think.there.is.consensus?>
        (commit "cabba9e..."))
   [...]))

-- would the commit need to be let-bound here?

Greetings,
Maxime

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

Re: New review checklist

[tanguy]

Dear Liliana,


On 2022-04-01 04:14, Liliana Marie Prikler wrote:
> in the sequel find the new review checklist, effective immediately.
> Failure to apply it will result in the confiscation of your machine for
> the purpose of making it usable for continuous integration.

The last patch I submitted seems to tick 90% of the boxes! ^_^'
So… 1) I'm still a WIP and 2) thanks again for taking the time to review 
it anyway! ;-)

Best regards,

-- 
Tanguy

Re: New review checklist

[Jonathan McHugh]

There is no reference to XML. Nor does it provide any interopabilietie with SOAP.
Please stop wasting the mailinglists time with this non XML based hokum, this is Guxi.FFSaek.

:)

====================
Jonathan McHugh
indieterminacy@libre.brussels

April 1, 2022 6:15 AM, "Liliana Marie Prikler" <liliana.prikler@gmail.com> wrote:

> Dear reviewer,
> 
> in the sequel find the new review checklist, effective immediately. 
> Failure to apply it will result in the confiscation of your machine for
> the purpose of making it usable for continuous integration.
> 
> Happy April Fools
> 
> -----
> 
> So you want to package a
> [ ] C [ ] C++ [ ] C# [ ] Common Lisp [ ] Emacs Lisp [ ] Fortran
> [ ] Guile [ ] Haskell [ ] Java [ ] Javascript [ ] Julia [ ] Nim
> [ ] OCaml [ ] Python [ ] R [ ] Rust [ ] V [ ] Zig [ ] ________
> application/library/________.
> It won't be added to Guix. Here's why it won't.
> 
> You appear to believe that
> [ ] linter warnings can easily be ignored
> [ ] `make check' does not need to succeed
> [ ] nobody will ever want to build your package on
> [ ] x86_64 [ ] i686 [ ] aarch64 [ ] armhf [ ] mips____
> [ ] powerpc____ [ ] riscv__ [ ] ______-mingw32 [ ] the Hurd
> [ ] commit hashes make for good version numbers
> [ ] hard-coding the commit field is a good idea
> [ ] using trivial-build system is a good idea
> [ ] we hard-code
> [ ] invocations of command line tools
> [ ] shared libraries
> [ ] _________
> for fun
> [ ] updating ______ to add your package does not cause a world rebuild
> [ ] committers have nothing better to do than trailing a branch that
> receives _____ commits per day.
> 
> Sadly your patch has/lacks
> [ ] copyright headers
> [ ] changes in other parts of the file
> [ ] indentation
> [ ] speling misstakes
> [ ] new-style inputs
> [ ] propagated inputs
> [ ] a useful synopsis
> [ ] a meaningful description
> [ ] a valid home-page
> [ ] correct licensing information
> [ ] significant improvements over the three other patches adding this  
> package, which themselves are stuck in review hell
> 
> The following technophilosophical objections also apply:
> [ ] the GNU FSDG prohibit _____________________________
> [ ] your package bundles a meaningless copy of
> [ ] ffmpeg
> [ ] v8
> [ ] font-awesome
> [ ] bundler
> [ ] rustc
> [ ] ________
> [ ] your package bootstraps itself using a sparse autoencoder trained
> on /dev/urandom
> [ ] your package is not reproducible thanks to
> [ ] embedded timestamps
> [ ] CPU feature detection during configure/compile time
> [ ] a flaky test suite
> [ ] an evil hack to call rand() inside a constexpr context
> [ ] Guix should not have to carry every fork of suckmore tools
> 
> In conclusion, this is what I think of you:
> [ ] Your patch looks good, but I'm not going to push it.
> [ ] Your patch would need some work, and I'm not going to invest that
> time on your behalf.
> [ ] Your patch is bad and you should feel bad for submitting it.
> [ ] Maintaining this package in your own channel is an adequate
> punishment for writing it.

Re: New review checklist

[Liliana Marie Prikler]

Hi Maxime,

Am Freitag, dem 01.04.2022 um 08:30 +0200 schrieb Maxime Devos:
> Liliana Marie Prikler schreef op vr 01-04-2022 om 06:14 [+0200]:
> > It won't be added to Guix.  Here's why it won't.
> > You appear to believe that
> > [...]
> > [ ] hard-coding the commit field is a good idea
> 
> Does the following count:
> 
> (define-public foo
>   (package
>     (name "foo")
>     (version "1.0.0")
>     (source
>       (origin
>         [...]
>         (file-name (git-file-name name version))
>         ;; Upstream does not tag versions, see
>         ;; <https://foo.bar/versions> for which commit
>         ;; corresponds to which version.
>         ;;
>         ;; (alternatively)
>         ;;
>         ;; Upstream deletes old tags every N months,
>         ;; so explicitly write the commit here.;
>         ;;
>         ;; (alternatively)
>         ;;
>         ;; There is consensus that the benefits of explicit commits
>         ;; outweigh the downsides, see
>         ;; <https://actually.I.dont.think.there.is.consensus?>
>         (commit "cabba9e..."))
>    [...]))
> 
> -- 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.

In the particular case of Minetest, another plausible style would be
(let ((commit ...)
      (revision ACTUAL-REVISION-OR-#f))
  (package 
    ...
    ;; tagged TAG as of DATE
    ;; For a complete list of versions, see <LINK>
    (version "just the version") 
    (source (origin ...
              ;; Upstream is known to yank tags, so always use commits.
              (commit commit)))))

Cheers

Re: New review checklist

[Maxime Devos]

[-- Attachment #1: Type: text/plain, Size: 2005 bytes --]

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.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

Re: New review checklist

[Tobias Geerinckx-Rice]

>-- would the commit need to be let-bound here?

You seem to already know the answer, which is 'no'.

Aside, 'let-binding' is not the opposite of 'hard-coding'.


Kind regards,

T G-R

Sent on the go.  Excuse or enjoy my brevity.

Re: New review checklist

[Liliana Marie Prikler]

Hi Maxime,

Am Freitag, dem 01.04.2022 um 19:46 +0200 schrieb Maxime Devos:
> [...]
> 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?
The main goal of let-binding commit and revision is to allow for easier
change.  Suppose you need to reference some half-release for some
obscure reason, then this style makes it easier to switch to what is
already established praxis.

In general, consider the poor soul who may have to read and maintain
your code after you get hit by a car because neither busses nor trams
run in your region.

Re: New review checklist

[Bengt Richter]

Hi Liliana, Maxime, et al,

On +2022-04-01 20:25:37 +0200, Liliana Marie Prikler wrote:
> Hi Maxime,
> 
> Am Freitag, dem 01.04.2022 um 19:46 +0200 schrieb Maxime Devos:
> > [...]
> > 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 a 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

Here ISTM important to understand exactly what you mean by "let-binding"
so I would really like it if I could type

--8<---------------cut here---------------start------------->8---
    info guile let-binding
--8<---------------cut here---------------end--------------->8---

into my command line interpreter, and get right to the details
as I often can for other things, e.g.

--8<---------------cut here---------------start------------->8---
    info guile define-macro
--8<---------------cut here---------------end--------------->8---

This suggests to me something like a translation project, except
not bewween local natural languages, but between
expert guile/guix implementer's terminology and detailed explanations
that various level programmers can go into as deeply as they want (following
suggested reading if not included in the info doc itself).

I am also imagining info could be instrumented to emit a minimal email
to a server that could accumulate statistics on no-hit lookups,
and that this could be visible in some tool when someone
feels like contributing to make

--8<---------------cut here---------------start------------->8---
    info guile what-does-this-mean
--8<---------------cut here---------------end--------------->8---

produce a result that we can all refer to when we want/need
to say "that's what I am talking about."

After getting past foot-binding and other bindings, wikipedia
got me to [0], which might be a nice read-further hint, but
what did Maxime mean, out of all those possibilities?

[0]    https://en.wikipedia.org/wiki/Scope_(computer_science)

To be really precise, if he could point to a formal definition
in some style from [1]

[1]    https://en.wikipedia.org/wiki/Denotational_semantics

that could really nail it when such detail was necessary.

Of course,

--8<---------------cut here---------------start------------->8---
     info guile define-language
--8<---------------cut here---------------end--------------->8---

leads to wonder-land. But if you just want a quick check
that you have the right concept for something you read,
well, good luck in RL -- in fact I just missed a local
library music event because I was reading guile info docs to
make examples for this post -- i.e. got drawn into reading
about define-language and not watching the time ;-/

I think for best effect, there should be no dependencies to prevent
usage anywhere "info guile" answers usefully at the command line.

Anyone else want to know exactly what Maxime meant by "let-binding" ? :)

> > 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
> > 
> > (packaeg
> >   (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?
> The main goal of let-binding commit and revision is to allow for easier
> change.  Suppose you need to reference some half-release for some
> obscure reason, then this style makes it easier to switch to what is
> already established praxis.
> 
> In general, consider the poor soul who may have to read and maintain
> your code after you get hit by a car because neither busses nor trams
> run in your region.
> 

-- 
Regards,
Bengt Richter

Re: New review checklist

[Liliana Marie Prikler]

Am Samstag, dem 02.04.2022 um 15:38 +0200 schrieb Bengt Richter:
> Hi Liliana, Maxime, et al,
> [...]
> Here ISTM important to understand exactly what you mean by "let-
> binding" so I would really like it if I could type
> 
> --8<---------------cut here---------------start------------->8---
>     info guile let-binding
> --8<---------------cut here---------------end--------------->8---
Does info '(guile)Local Bindings' answer this question? :P
We might want to make it so that '(guile)let' shows this node
otherwise.

> into my command line interpreter, and get right to the details
> as I often can for other things, e.g.
> 
> --8<---------------cut here---------------start------------->8---
>     info guile define-macro
> --8<---------------cut here---------------end--------------->8---
> 
> This suggests to me something like a translation project, except
> not beween local natural languages, but between
> expert guile/guix implementer's terminology and detailed explanations
> that various level programmers can go into as deeply as they want
> (following suggested reading if not included in the info doc itself).
> 
> I am also imagining info could be instrumented to emit a minimal
> email to a server that could accumulate statistics on no-hit lookups,
> and that this could be visible in some tool when someone
> feels like contributing to make
I don't think let-binding is such a complicated concept.  It is exactly
what it says on the tin: Using the let syntax to bind a variable. 
Furthermore, even if you don't understand the term on its own, Maxime
went on to state the question at hand in more detail, from which you
could infer the difference between "let-binding" and not "let-binding".


Cheers