Hi Aymeric,
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."
Aymeric Agon-Rambosson <aymeric.agon@yandex.com> writes:
> Le mercredi 4 janvier 2023 à 13:13, Nicholas D Steeves
> <sten@debian.org> a écrit :
>
>> ca55bc8: Nice find, and fix, thank you! (I haven't tested it
>> yet, but
>> assume you have, that it's good, and that it will pass when I
>> test it)
>
> I have, and I have checked that the generated HTML documentation
> is the same as in currently installed versions of yasnippet (from
> what I can tell, only the order of the documented symbols in the
> documentation changes, which could be related to changes in the
> way org export works). If you think the patch is good, I'll
> forward it upstream and explain my thinking.
>
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).
>> b9ccd2b: What's the purpose of this digression from the way
>> dh_elpa does
>> things? It looks like it introduces potential indeterminacy.
>> At a
>> minimum, an explanation needs to be provided.
>
> I just modeled the override according to what I did in my other
> packages (see vertico or consult, for instance), for which I had
> no objection. But it turns out that dh_elpa does "emacs -Q -L
> /path/to/needed/elpa/src/" rather than "emacs -q". The second one
> seemed simpler, but they are not entirely equivalent, the debian
> site-files do a little bit more than just adding directories to
> the load-path. I will remove this commit.
>
Thanks! [edit: I've reverted on my branch]
>> 0a76135: I will not sponsor due to this action, because it's not
>> ok
>> to disable all 91 tests, when only 7 are failing
>> https://ci.debian.net/data/autopkgtest/unstable/amd64/y/yasnippet/28997908/log.gz
>>
>> Feel skip these seven tests using another method, but please say
>> why
>> this is the correct action.
>
> The thinking was the following :
>
> The tests' failing has nothing to do with us, a simple "rake
> tests" on the upstream repo fails the same, and upstream seems
> unconcerned about this. In fact, there is a PR upstream (#1125)
> that solves some of the failing tests that hasn't been merged in
> more than a year.
>
It sounds like a new upstream [co]maintainer is be needed.
> I would have liked the failing tests not to prevent the build from
> succeeding, but still be able to see that there are some
> failing. By disabling dh-elpa-test, we make the build succeed, but
> autopkgtest still runs after the build, and the failure can still
> be visible on ci.debian.net (which is good). If we skip the tests,
> we're making them disappear from the build and from autopkgtest.
>
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.
> I have neither the time nor the inclination to investigate the
> failing tests.
Fair point. Thanks for mentioning upstream PR. I've imported it.
Cheers,
Nicholas
Attachment:
signature.asc
Description: PGP signature