Re: yasnippet commits review

[Aymeric Agon-Rambosson <aymeric.agon@yandex.com>]

Hello Nicholas,

Le dimanche  8 janvier 2023 à 18:12, Nicholas D Steeves 
<sten@debian.org> a écrit :

> I've created the branch "temp-agon-reviewed_by_sten" which is
> fast-forwardable relative from "temp".  This was necessary 
> because the
> following are not doc-related...self tests for core 
> functionality
> shouldn't break:
>
>     3 unexpected results:
>        FAILED  basic-jit-loading
>        FAILED  basic-jit-loading-with-compiled-snippets
>        FAILED  visiting-compiled-snippets
>
> (ert-deftest basic-jit-loading ()
>   "Test basic loading and expansion of snippets"
> …
> (ert-deftest basic-jit-loading-with-compiled-snippets ()
>   "Test basic loading and expansion of compiled snippets"
> …
> (ert-deftest visiting-compiled-snippets ()
>   "Test snippet visiting for compiled snippets."

Very well, I've seen the branch. Since we can rewrite history as 
long as we haven't merged into master, will you let me remove my 
commits from your branch rather than simply revert them ? I can do 
it whenever its convenient for you, just before you merge to 
master.

> This patch has "Forwarded: yes" in the header, so you've already 
> claimed
> that it's been forwarded ;) It would be nice to have the URL it 
> was/will
> be forwarded to rather than "yes" here, so that the next person 
> who
> works on this package can track upstream discussion and status 
> of your
> patch.  I think it's likely upstream will accept it, an in the 
> interim I
> think other people will appreciate having it made public.  Ie 
> makes it
> easier for someone to fork and apply all the important PRs to 
> get
> yasnippet working again.
>
> Also if for some reason the upstream project becomes 
> inaccessible, it's
> worthwhile to have a line or two in your patch header that 
> explains your
> thinking (for the future maintainer of the Debian package). 
> Sometimes
> the Debian packages ends up being the only living project, and 
> thus the
> defacto "upstream" (hopefully only until an upstream fork is 
> made).

I haven't forwarded it yet, this is exactly what I wanted to do 
after you reviewed the patch. I'll open a PR on the upstream repo, 
reference the issue David opened, explain my thinking there and on 
the patch header as well.

> Unfortunately this Debian package doesn't have an active human
> maintainer, so this becomes an "if someone happens to notice and 
> care
> about the failure" rather than the existing early warning 
> system.  The
> hard failure alerts team members and interested parties, and 
> correctly
> removes the package from testing.
>
> Fair point.  Thanks for mentioning upstream PR.  I've imported 
> it.

Sorry for this. What I thought would be a (not so) simple unbreak 
of the documentation build turned out to be hiding other 
errors. I've seen that you and David discuss it on 
#debian-emacsen, I'll try and look into the remaining tests this 
weekend if I have the time.

Best,

Aymeric