Subject: bug#69666: [PATCH] (vtable-update-object): Make old-object argument optional

From: Eli Zaretskii <[email-masked]>
Date: Sat, 16 Mar 2024 12:29:59 +0200

> Date: Fri, 15 Mar 2024 19:41:33 -0500
> Cc: [email-masked]
> From: Adam Porter <[email-masked]>
> 
> > Thanks, I have some minor comments below.
> 
> Thanks for your review.  Please see the attached patch which addresses 
> those three items and is rebased on current master.  Please let me know 
> if I need to make any further changes.

Thanks, this LGTM.  Just a couple of minor nits:

> Subject: [PATCH] (vtable-update-object): Make old-object argument optional
>  (Bug#69666)

This is too long for the generated ChangeLog.  Since this just
reiterates what the log message below it says, I suggest:

  . make the heading more terse, like

     'vtable-update-object' can now be called with one argument

  . move the bug number to the actual log entry:

    * lisp/emacs-lisp/vtable.el (vtable-update-object): Make 'old-object'
    argument optional.  (Bug#69666)

> +(defun vtable-update-object (table object &optional old-object)
> +  "Update OBJECT's representation in TABLE.
> +When OLD-OBJECT is non-nil, replace OLD-OBJECT with OBJECT and display

"When" has a meaning of time, which is not what you mean here.  I
suggest to use "If" in these cases.