diff mbox series

[bug#77258] New package: emacs-boxy

Message ID 87o6xp571l.fsf@gmail.com
State New
Headers
Series [bug#77258] New package: emacs-boxy |

Commit Message

Amy Grinn March 25, 2025, 5:55 p.m. UTC 
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!

Comments

Ian Eure March 29, 2025, 3:49 p.m. UTC | #1 
Hi Amy,

Amy Grinn <grinn.amy@gmail.com> 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 <grinn.amy@tuta.com>
> 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
Thompson, David April 3, 2025, 3:04 p.m. UTC | #2 
Hi Amy,

The latest version of your patch looks good overall. Thanks for
expanding the description and adding a copyright line. As you
mentioned, many packages in emacs-xyz.scm use tarballs from ELPA so
given the instability of the upstream repo due to hosting migrations I
think using an ELPA URL is just fine here.

FWIW, patches as attachments are no problem for me. Whether inline or
attachment, doing code review by email is just as frustrating. ;)

Also, I think the comments from Ian about the code in the project
itself are irrelevant to the discussion of this patch, which should be
scoped to just the package recipe.

I modified the hash to use the conventional base32 format and pushed to master.

Congrats on your first Guix patch!

- Dave
Amy Pillow April 4, 2025, 9:29 a.m. UTC | #3 
> Hi Amy,
> 
> The latest version of your patch looks good overall. Thanks for
> expanding the description and adding a copyright line. As you
> mentioned, many packages in emacs-xyz.scm use tarballs from ELPA so
> given the instability of the upstream repo due to hosting migrations I
> think using an ELPA URL is just fine here.
> 
> FWIW, patches as attachments are no problem for me. Whether inline or
> attachment, doing code review by email is just as frustrating. ;)
> 
> Also, I think the comments from Ian about the code in the project
> itself are irrelevant to the discussion of this patch, which should be
> scoped to just the package recipe.
> 
> I modified the hash to use the conventional base32 format and pushed to master.
> 
> Congrats on your first Guix patch!
> 
> - Dave

Thank you so much Dave, I appreciate it! I agree with everything you
said and I think both the ELPA src url and patches-as-attachments will
make it easier for me to contribute in the future.

I also agree that issues in the source project should be addressed
outside of the guix-patches mailing list. If anyone has an issue using
the boxy library please use the forge's issue tracker (currently gitlab)
instead of Guix'. That way, Emacs users who don't use Guix will be able
to join the discussion.
diff mbox series

Patch

From b3223cec32425d6b8cf2c11545ed63ad6dabae88 Mon Sep 17 00:00:00 2001
From: Amy Grinn <grinn.amy@tuta.com>
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"))
+	      (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.")
+    (license license:gpl3+)))
+
 (define-public emacs-buffer-env
   (package
     (name "emacs-buffer-env")
-- 
2.49.0