From: Eli Zaretskii <eliz@gnu.org>
To: Stefan Kangas <stefan@marxist.se>
Cc: 37410@debbugs.gnu.org
Subject: bug#37410: [PATCH] Several doc fixes in package.el
Date: Sun, 15 Sep 2019 19:37:14 +0300 [thread overview]
Message-ID: <83a7b5y0qt.fsf@gnu.org> (raw)
In-Reply-To: <CADwFkmkRnX14EHDvBU1eLLAHXTwcu6kUTDT0wkr8j+yTDXrxXg@mail.gmail.com> (message from Stefan Kangas on Sun, 15 Sep 2019 18:01:17 +0200)
> From: Stefan Kangas <stefan@marxist.se>
> Date: Sun, 15 Sep 2019 18:01:17 +0200
>
> Seeing as the documentation in package.el leaves much to be desired, I
> spent some time adding doc strings and fixing checkdoc and stylistic
> errors. I've attached a patch with my results, which should improve
> the situation a little bit at least.
Thanks for working on this.
> (defun package-check-signature ()
> "Check whether we have a usable OpenPGP configuration.
> -If true, and `package-check-signature' is `allow-unsigned',
> -return `allow-unsigned', otherwise return the value of
> -`package-check-signature'."
> +If true, and variable `package-check-signature' is
"True" is inappropriate here. I'd use "If so, and..." instead.
> (defun package--from-builtin (bi-desc)
> + "Create a `package-desc' object from BI-DESC.
> +BI-DESC should be an `package--bi-desc' object."
^^
"a"
> (defun package-desc-full-name (pkg-desc)
> + "Return full name of package-desc object PKG-DESC.
> +For example, if the package is named \"foo\" and has version
> +\"1.2.3\", then return \"foo-1.2.3\"."
Instead of "for example", it is better to tell explicitly whet this
does. E.g.,:
"Return full name of package-desc object PKG-DESC.
This is the name of the package with its version appended."
> (defun package-desc-suffix (pkg-desc)
> + "Return suffix of package-desc object PKG-DESC.
I'd say "file-name extension" instead of "suffix".
> (defun package-desc--keywords (pkg-desc)
> + "Return keywords of package-desc object PKG-DESC."
Suggest to say something about what these keywords are and where they
come from. Otherwise, "keywords" is such a vague term that it's
impossible to understand that without reading the code.
> +Convert EXP into a `package-desc' object using the
> +`package-desc-from-define' constructor before pushing it to
> +`package-alist.
^
A closing quote missing there.
> (defun package-archive-base (desc)
> - "Return the archive containing the package NAME."
> + "Return the archive containing the package DESC."
I'd say "the package described by DESC".
> (defun package-install-from-buffer ()
> - "Install a package from the current buffer.
> + "Install package from current buffer.
Why this change?
> ;;;###autoload
> (defun package-install-file (file)
> - "Install a package from a file.
> + "Install package from FILE.
And this?
> (defun describe-package-1 (pkg)
> + "Insert package description of PKG at point.
> +Helper function for `describe-package'."
The "at point" here is ambiguous: does it mean "insert at point" or
"PKG at point"?
> (defun package-install-button-action (button)
> + "Run `package-install' on package defined by BUTTON.
Can a package really be defined by a button?
> (defun package-keyword-button-action (button)
> + "Show *Packages* buffer filtered by keyword from BUTTON label.
*Packages* should be in double quotes.
I generally find this sentence confusing: what do you mean by "keyword
from BUTTON label"?
> +(defun package-make-button (text &rest properties)
> + "Insert button labelled TEXT with button PROPERTIES at point.
^^^^^^^^
"labeled"
> (defun package--print-email-button (name)
> + "Insert a button to email NAME at point.
"To email NAME" is confusing. I'd suggest to rename it ADDRESSEE.
"Insert a button to email" is also confusing. Is this alternative
correct?
Insert a button whose action will send email to ADDRESSEE.
> +NAME should have the form (FULLNAME . EMAIL) where NAME is either
^^^^
FULLNAME
> (defvar package-list-unversioned nil
> - "If non-nil include packages that don't have a version in `list-package'.")
> + "If non-nil, include packages that don't have a version in `list-package'.")
^^^^^^^^^^^^
"list-packages", I presume?
> (defvar package--emacs-version-list (version-to-list emacs-version)
> - "`emacs-version', as a list.")
> + "Variable `emacs-version' as a list.")
"The value of `emacs-version', as a list."
next prev parent reply other threads:[~2019-09-15 16:37 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-15 16:01 bug#37410: [PATCH] Several doc fixes in package.el Stefan Kangas
2019-09-15 16:37 ` Eli Zaretskii [this message]
2019-09-15 19:56 ` Stefan Kangas
2019-09-21 21:14 ` Stefan Kangas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=83a7b5y0qt.fsf@gnu.org \
--to=eliz@gnu.org \
--cc=37410@debbugs.gnu.org \
--cc=stefan@marxist.se \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).