Subject: Re: bug#28803: [PATCH] Fixed compiler warnings for advised functions.

From: John Williams <[email-masked]>
Date: Sat, 14 Oct 2017 17:30:46 -0700

On Sat, Oct 14, 2017 at 4:47 PM, Noam Postavsky
<[email-masked]> wrote:
> John Williams <[email-masked]> writes:
>
>> Oops. Is there anything that can be salvaged from my patch? Aside
>> from fixing the bug, it also adds a unit test and refactors the logic
>> for finding a function's argument list into a separate function
>> that's not part of the help system.
>
> We could add the test, it seems to be passing in emacs-26.  Have you
> assigned copyright to Emacs?  (It's okay if you haven't, the patch is
> small enough to go in regardless, we would just need to mark it.)

No; how would I go about doing that? The organization of the dev site
is a bit confusing to me.

> I don't think there's much need for moving the function argument
> retrieval out of the help system.

It's not a huge deal to me, but it seems weird that something like
nadvice.el would depend on the help system (which it would in my patch
if I hadn't moved that function--I assume the fix that was already
committed does something similar).

> (By the way, if you've spent some time looking at help-function-arglist,
> perhaps you have some ideas about Bug#26270?
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=26270)

I took a look at your patch, and without having tried running it, it
seems pretty reasonable to me. A unit test of some sort would be nice,
but since there don't seem to be any for help-function-arglist yet,
accepting the patch as-is would leave the unit test situation no worse
than it was before. I'm not a regular contributor, though, so my
opinion is worth what you paid for it.