Subject: Re: [bug#53535] [PATCH] gnu: Add emacs-popper

From: Nicolas Goaziou <[email-masked]>
Date: Fri, 28 Jan 2022 09:11:39 +0100

Hello,

Niklas Eklund <[email-masked]> writes:

> this patch adds the emacs-popper package.

Thank you. Somme comments follow..

> Subject: [PATCH] gnu: Add emacs-popper.

The commit message is missing a line:

  * gnu/packages/emacs-xyz.scm (emacs-popper): New variable.

> +(define-public emacs-popper
> +  ;; No tagged release upstream for version 0.45

This is true, but the commit matching release 0.45 is
"851d83882192ac9599ac5b053614a42d683b3fab", not the one you're using.

> +  (let ((commit "527a85c49174e6e79220f0ed0761c204a979eae6") (revision "0"))

Could you add a newline between (commit ...) and (revision ...)?
> +    (package
> +      (name "emacs-popper")
> +      (version "0.45")

You bound revision, but you're not actually making use of it. I think
there are two possibilities here:

- if you use commit 851d83882192ac9599ac5b053614a42d683b3fab, you can
  keep using (version "0.45"), and explain in a comment that the chosen
  commit matches version bump;

- if you use another commit, you need to use (git-version "0.45"
  revision commit) instead, and explain in a comment why you're using an
  untagged commit.

Could you send an updated patch?

Regards,
-- 
Nicolas Goaziou