Subject: Re: [bug#63545] [PATCH 1/3] gnu: heimdal: Build from Git.

From: Josselin Poiret <[email-masked]>
Date: Wed, 17 May 2023 22:20:19 +0200

Hi Felix,

I am no user of Heimdal, but this looks good to me apart from some
nitpicks below.

Felix Lechner via Guix-patches via <[email-masked]> writes:

>  (define-public heimdal
> +  (let ((commit "a6cf94577c0d1e5bca5304342e4ddffb18255afe")
> +        (revision "1"))

No need to use commit+revision for fixed releases, unless upstream has
the bad habit of moving their upstream tags. I see the tag
heimdal-7.8.0, which you should use in (commit ...), possibly with
(commit (string-append "heimdall-" version)).

>    (package
>      (name "heimdal")
> -    (version "7.8.0")
> +    (version (git-version "7.8.0" revision commit))

This would lead to a very ugly version string for what is actually
7.8.0, no need to change this.

The other patches seem fine, although I think you don't need to go as
far as make the indentation change its own patch (and by dropping the
commit+revision thing, there shouldn't be one).

I haven't tried building this yet though, will let QA go over it.

Best,
-- 
Josselin Poiret