Re: ELPA submission: plz-see

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Augusto Stoffel <arstoffel@gmail.com>
>> Date: Mon, 30 Oct 2023 08:17:30 +0100
>> 
>> I would like to submit the following package to ELPA:
>> 
>>   https://github.com/astoff/plz-see.el

Loks good, I only have a few comments:

diff --git a/plz-see.el b/plz-see.el
index 2e5451f..74d62b1 100644
--- a/plz-see.el
+++ b/plz-see.el
@@ -4,6 +4,8 @@
 
 ;; Author: Augusto Stoffel <arstoffel@gmail.com>
 ;; Keywords: comm, network, http
+;; Version: 1.0.0
+;; Package-Requires: ((emacs "29.1"))
 
 ;; This program is free software; you can redistribute it and/or modify
 ;; it under the terms of the GNU General Public License as published by
@@ -20,10 +22,12 @@
 
 ;;; Commentary:
 
-;; 
+;; Please write a brief commentary section, that is useful when
+;; querying the package from a `describe-package' buffer.
 
 ;;; Code:
 
+(eval-when-compile (require 'cl-lib))
 (require 'json)
 (require 'plz)
 
@@ -57,7 +61,7 @@ If nil, never delete old response buffers."
 
 (defcustom plz-see-display-action nil
   "The ACTION argument `plz-see' passes to `display-buffer'."
-  :type 'sexp)
+  :type display-buffer--action-custom-type)
 
 (defcustom plz-see-header-line-format
   (let ((headers '(plz-see-header-line-status
@@ -181,7 +185,7 @@ call and AS specifies the argument type they expect."
                      ('response response)
                      ('buffer buffer)
                      ((or 'binary 'string 'file `(file ,_))
-                      (user-error "plz-see does not accept :as %s" as))
+                      (user-error "`plz-see' does not accept :as %s" as))
                      ((pred functionp)
                       (with-temp-buffer
                         (insert (plz-response-body response))

>> It is an interactive HTTP client in the sense that request responses are
>> pretty-printed in a pop-up buffer. It is useful e.g. to test REST APIs.
>
> I wonder if you could come up with a name that tells something about
> the package's purpose.  "plz-see" tells me "please see", and I don't
> see any relation to HTTP, REST, pretty-printing, or popup buffers.

I agree, from what I see this is just an implementation detail.  Plz is
a peculiar enough name as it is, it shouldn't be inherited by packages
that depend on it.  Or is there any reason why the package couldn't also
use url.el?

-- 
Philip Kaludercic