Mathieu Othacehe schreef op vr 18-06-2021 om 14:15 [+0200]:
> Hello Maxime,
> 
> > +      (`(,(or 'lambda 'lambda*) ,_ (invoke . ,_) . ,_)
> > +       (list (make-warning package
> > +                           ;; TRANSLATORS: check and #:tests? are a Scheme
> > +                           ;; symbol and keyword respectively and should not
> > +                           ;; be translated.
> > +                           (G_ "the 'check' phase should respect #:tests?")
> > +                           #:field 'arguments)))

I just noticed the following test case in (tests lint) is somewhat bogus:

> +              '((replace 'check+
> +                  (lambda (#:key tests? #:allow-other-keys?)

Instead of 'lambda', this should be 'lambda*'.

Also, the value for #:phases can now be a G-expression,
so the usage of 'package-arguments' in the patch would need to be adjusted
as well.

> I like the idea behind this patch. However I think the detection pattern
> could be improved for instance, here are a few unreported packages:
> 
> - dejagnu
> - python-dateutil
> - eigen
> 
> Maybe we should check directly if the tests? variable is used within the
> 'check replace phase?

So, basically, test if applying the following procedure to the body
succeeds?

(define (sexp-uses-tests?? sexp)
  (sexp-contains-atom? sexp 'tests?))

(define (sexp-contains-atom? sexp atom)
  ; atoms are compared with eq? and vectors are currently not supported
  (if (pair? sexp)
      (or (sexp-contains? sexp atom)
          (sexp-contains? sexp atom))
      (eq? sexp atom)))

That seems a good improvement for a v2.

Thanks,
Maxime.