GNU bug report logs - #72735
31.0.50; [PATCH] Make more bug-reference variables customizeable

Previous Next

Full log

Message #14 received at 72735 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Björn Bidar <bjorn.bidar <at> thaodan.de>
Cc: 72735 <at> debbugs.gnu.org
Subject: Re: bug#72735: 31.0.50;
 [PATCH] Make more bug-reference variables customizeable
Date: Tue, 20 Aug 2024 21:39:34 +0300

> Date: Tue, 20 Aug 2024 18:34:38 +0300
> From:  Björn Bidar via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> I noticed that some of the variables that can be modified
> by the user or as mentioned as such were not defined as custom variable.
> These patches to so and correct the group of one of the existing
> defcustom.
> 
> The patches don't change any of the existing functionality
> but only document these variables better and make them easier to modify.
> Because of that I set version to Emacs 30.1. Please tell if that's ok.

The emacs-30 release branch is closed to enhancements, it only
receives bugfix changes and improvements in documentation.  Converting
a defvar to a defcustom is not just a documentation change, it
radically changes how the variable is initialized and set.  So this is
not appropriate for the release branch.

Please see several comments below.

> * lisp/progmodes/bug-reference.el (bug-reference-url-format): Define as
> custom type. The manual and the documentation string talk like the
             ^^
> variable like is a custom variable. It does make sense to define
                                    ^^
Our convention is to leave two spaces between sentences in comments,
strings, and commit log messages.  There are several places in the
patch where you left only one space.

> -(defvar bug-reference-url-format nil
> +(defcustom bug-reference-url-format nil
>    "Format used to turn a bug number into a URL.

I wonder how this makes sense as a defcustom, since this variable must
be file-local, AFAIU.  In any case, the doc string should explain the
semantics of the nil value.

> + :group 'bug-reference)

This is redundant (here and elsewhere in the patches), since the
package name will supply the group.

> +FORGE-TYPE is the type of the forge, e.g., gitlab, gitea,
> +sourcehut, or github.

This leaves it unsaid how these symbols are used, and what symbols are
recognized.

Finally, I think this warrants a NEWS entry.

Thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.