Drew Adams <drew.adams@oracle.com> writes:

> I think the first line of the doc string should
> say what a true value means, because the option
> name does that.

> Or at least the doc string should somehow make
> clear which Boolean value means which behavior.

> E.g.:

> Non-nil means do not prompt for confirmation when reverting a help
> buffer.

I agree this is better and also matches the recommendation in
`(elisp)Documentation Tips', I've updated the patch to reflect this.


Stefan Kangas <stefan@marxist.se> writes:

> Your patch seems to be lacking ChangeLog entries as described in
> etc/CONTRIBUTE.  You would help us if you could add such entries,
> especially if you are planning to do more changes.

I've updated the commit message per the CONTRIBUTE file, let me know if
there are any changes needed.

As a side note, it may be helpful to mention this file in the
`(emacs)Sending Patches for GNU Emacs' info node.  That was the page I
was referencing for info before submitting this.

>> +(defcustom help-mode-revert-buffer-noconfirm nil
>
> This would change the default, right?  Any rationale for that?

The default should be the same.  I've updated the docstring which I
think makes this clearer.

>> +  "Indicates whether to prompt for confirmation when reverting a
>> +help buffer."
>
> The first sentence of a doc string should fit on one line.

Updated with Drew's suggestion.

>>  (defun help-mode-revert-buffer (_ignore-auto noconfirm)
>> -  (when (or noconfirm (yes-or-no-p "Revert help buffer? "))
>> +  "Revert help-mode buffer.  See
>> +`help-mode-revert-buffer-noconfirm' to control whether user is
>> +prompted for confirmation."
>
> The first line should be one sentence only, and the second sentence
> start on the second line.

Updated to match these conventions.

Thank you both for the feedback, let me know if there any other changes
that should be made.