Mathieu Othacehe schreef op di 29-06-2021 om 12:34 [+0200]:
> Hello Maxime,
> 
> Thanks for the new revision.
> 
> > +@deffn {Procedure} gexp->approximate-sexp @var{gexp}
> > +Sometimes, it may be useful to convert a G-exp into a S-exp.
> > +For example, some linters (@pxref{Invoking guix lint})
> 
> You can write longer sentences here, up to 78 columns.  If you are using
> Emacs, fill-paragraph does the right thing.

I did a "fill-paragraph" in the v3.

> > +  (define (sexp-uses-tests?? sexp)
> > +    "Test if SEXP contains the symbol 'tests?'."
> > +    (sexp-contains-atom? sexp 'tests?))
> > +  (define (sexp-contains-atom? sexp atom)
> > +    "Test if SEXP contains ATOM."
> > +    (if (pair? sexp)
> > +        (or (sexp-contains-atom? (car sexp) atom)
> > +            (sexp-contains-atom? (cdr sexp) atom))
> > +        (eq? sexp atom)))
> 
> It would make more sense to define "sexp-uses-tests??" later as it uses
> "sexp-contains-atom" that is defined afterwards.

Indeed. I switched these two procedures around in the v3.

> > +       (or (check-phases-delta head)
> > +           (check-phases-deltas tail)))
> 
> I think it should be "append" instead of "or". Otherwise, it fails to
> detect package which 'replace is not the first phase, see mkvtoolnix for
> instance.

Indeed. I added a test case and replaced "or" with "append". The linter
now detects about 300 additional cases.

Greetings,
Maxime.