[bug#74979] [PATCH 0/4] scripts: style: Sort more kinds of package

["Ludovic Courtès" <ludo@gnu.org>]

Hi Herman,

Herman Rimm <herman@rimm.ee> skribis:

> The warnings added in [PATCH 4/4] are emitted multiple times.  How
> should I prevent that?  Or should I put them behind a --verbose option?

Not sure, do you have an example on how to trigger it?

> +++ b/guix/scripts/style.scm
> @@ -508,9 +508,15 @@ (define (order-packages lst)
>           (((or 'package 'package/inherit) fields ...)
>            (let ((name (and=> (assoc-ref fields 'name) first))
>                  (version (and=> (assoc-ref fields 'version) first)))
> +            (if (and name version)
> +                (unless (and (string? name) (string? version))
> +                  (warning (G_ "non-string name/version for ~a~%") pkg))
> +                (warning (G_ "package fields not found for ~a~%") pkg))
>              (values name version)))
> -         (_ (and (values #f #f)))))
> -      (_ (and (values #f #f)))))
> +         (_ (and (warning (G_ "package record missing for ~a~%") pkg)
> +                 (values #f #f)))))
> +      (_ (and (info (G_ "not sorting top-level S-exp.: ~a~%") pkg)
> +              (values #f #f)))))

You shouldn’t rely on the return value of ‘info’, ‘warning’, etc.:
they’re not specified (that’s generally the case for procedures called
for side effects only).

So I’d recommend:

  (begin
    (info …)
    (values #f #f))

(You don’t even need ‘begin’ in this context.)

The other patches LGTM, though perhaps ‘tests/guix-style.sh’ could be
augmented a bit to cover the new cases?

Thanks,
Ludo’.