Subject: Re: [bug#77258] [PATCH] New package: emacs-boxy

From: Ian Eure <[email-masked]>
Date: Sat, 29 Mar 2025 08:49:37 -0700

Hi Amy,

Amy Grinn <[email-masked]> writes:

> Hello,
>
> I would like to add my package, emacs-boxy, to Guix.
>
> This is my first patch to Guix so let me know if I did anything 
> wrong!

Thank you for contributing to Guix!  Please see 
(guix)Contributing[1] for info about sending patches.  The Guix 
tooling expects patches to be submitted with `git send-email', and 
not mailed as an attachment.

Other comments below.

>>>From b3223cec32425d6b8cf2c11545ed63ad6dabae88 Mon Sep 17 
>>00:00:00 2001
> From: Amy Grinn <[email-masked]>
> Date: Tue, 25 Mar 2025 18:17:25 +0100
> Subject: [PATCH] gnu: Add emacs-boxy
>
> * gnu/packages/emacs-xyz.scm (emacs-boxy): New variable.
> ---
>  gnu/packages/emacs-xyz.scm | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/gnu/packages/emacs-xyz.scm 
> b/gnu/packages/emacs-xyz.scm
> index f00c5e4b29..0423858b2f 100644
> --- a/gnu/packages/emacs-xyz.scm
> +++ b/gnu/packages/emacs-xyz.scm
> @@ -39025,6 +39025,21 @@ (define-public emacs-boxquote
>  example code.")
>      (license license:gpl3+)))
>  
> +(define-public emacs-boxy
> +  (package
> +    (name "emacs-boxy")
> +    (version "2.0.0")
> +    (source (origin
> +	      (method url-fetch)
> +	      (uri (string-append 
> "https://elpa.gnu.org/packages/boxy-"
> +                                  version ".tar"))

Please prefer the upstream Git repo over M/ELPA.

> +	      (sha256 (base64 
> "6w1QtINpWz9voCrc/ne6q8sPj4UcA3s8E/f/Pebjz+0="))))
> +    (build-system emacs-build-system)
> +    (home-page "https://gitlab.com/grinn.amy/boxy")
> +    (synopsis "A boxy layout framework")
> +    (description "Boxy provides an interface to create a 3D 
> representation of boxes.")

"Boxy" should probably reference the package name 
(@code{emacs-boxy}), "This package," or similar.  It would also be 
nice to have a bit more info about what the package is used for.

I also noticed a couple issues with the package itself which ought 
to be corrected:

- Your `defcustom' and `defface' forms don’t have `:group boxy', 
  so won’t appear if you `M-x customize-group RET boxy RET'.
- Several macros from `cl-macs' are used, but `cl-macs' isn’t 
  required.  ex. `cl-defstruct', `cl-flet*', possibly others. 
  `cl-lib' might also require `cl-macs', but you should always 
  require your direct dependencies instead of relying on 
  transitive ones.
- You may want to condsider using symbols or keywords for 
  `boxy-relationships' instead of strings.

Thanks,

  -- Ian

[1]: 
https://guix.gnu.org/manual/devel/en/html_node/Sending-a-Patch-Series.html#Single-Patches-1