Subject: Re: [bug#42216] [PATCH] gnu: Add emacs-flycheck-ledger.

From: Nicolas Goaziou <[email-masked]>
Date: Mon, 06 Jul 2020 14:24:47 +0200

Hello,

[email-masked] writes:

> * gnu/packages/emacs-xyz.scm (emacs-flycheck-ledger): New variable.

Thank you. Some comments follow.

> +(define-public emacs-flycheck-ledger
> +  (package
> +    (name "emacs-flycheck-ledger")
> +    (version "20200304.2204")

There's a stable "0.5" release. We should use it instead.

> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append "https://melpa.org/packages/flycheck-ledger-"
> +                                  version ".el"))

We don't use source files from MELPA, as they are somewhat modified
in-place. Use git-fetch instead.

Also, could you move `origin' behind `source'?

> +              (sha256
> +               (base32
> +                "036xn52xjxbbvag9slqi4xqlvzg9p2q544hhxhqqi7jz1kap0yx8"))))
> +    (inputs `(("ledger" ,ledger)))

I think this is not sufficient, because Emacs Flycheck Ledger will not
find the Ledger executable. You need to tweak
`flycheck-ledger-executable', using `emacs-substitute-variables'.

Could you send an updated patch?

Regards,
-- 
Nicolas Goaziou