Hi Nicholas, Thanks for your review! Please see my replies inline below.
Nicholas D Steeves <s...@debian.org> writes: > Xiyue Deng <manp...@gmail.com> writes: > > You're incorrect about: > >> * Drop unused override_dh_auto_test in d/rules (dh_elpa_test is in >> effect) > > override_dh_auto_test: > @echo Using dh_elpa_test to run tests instead of upstream Makefile > > Used to be useful, because the Makefile tests used to run as well as the > dh_elpa_test ones. Dh_elpa_test was also "in effect then"... > I believe dh_elpa_test disables dh_auto_test by default unless it is disabled in debian/elpa-test[1]. I think you did this change back in this commit[2] back in 2019. On the other hand, it looks like dh_auto_test was disabled by dh-elpa even earlier[3]. Maybe you were referring to even more back then? Anyway, would be good to figure this out. >> * Update debian copyright year in d/copyright > > -Copyright: 2015-2017 David Bremner <brem...@debian.org> > +Copyright: 2015-2024 David Bremner <brem...@debian.org> > > In terms of copyright, please explain your understanding of copyright > what you're doing here. This is not a mere 's/17/24/' symbol > manipulation. > I was thinking about adding myself to copyright, but I remember one of the early reviews I got (which I forgot from whom) suggested that adding oneself to copyright list would require significant change to the debian/ files so I didn't do it and instead extending the copyright year coverage of the existing entry. Maybe in such case like this one, I should go over the git history and add all people that had contributed and also add myself to the list. Wdyt? >> Note: this revision drops the dependency on markdown, which is important >> so that itself and all its reverse dependencies are free from >> bug#1063645 so that they are free from being removed from testing. > > This is premature. Also, does upstream skip functional tests if a > $PATH/markdown binary cannot be called? If this is the case then the > correct action is to configure a $PATH/compatible-markdown-replacement. > I checked by a simple grep in tests/markdown-test.el. I had a closer look just now and it looks like it will consider 3 commands, `markdown', `pandoc', `markdown_py', and use one that is available[4]. As we already depends on pandoc, it will use it instead of markdown. That said, it would be better if we use an alternative markdown implementation to replace the out-of-date markdown, so I have now used discount to replace markdown and reworked the Build-Depends list in the source stanza[5], as well as the Recommends list in binary stanza[6]. I have also compared the output with or without markdown and with discount: With markdown dependency: ,---- | Ran 543 tests, 540 results as expected, 0 unexpected, 3 skipped (2024-06-11 04:04:39+0000, 1.465706 sec) | 1 expected failures | | 3 skipped results: | SKIPPED test-markdown-flyspell/check-word-p | SKIPPED test-markdown-flyspell/remove-overlay | SKIPPED test-markdown-flyspell/yaml-metadata `---- Without markdown dependency: ,---- | Ran 543 tests, 540 results as expected, 0 unexpected, 3 skipped (2024-06-11 03:56:33+0000, 1.504459 sec) | 1 expected failures | | 3 skipped results: | SKIPPED test-markdown-flyspell/check-word-p | SKIPPED test-markdown-flyspell/remove-overlay | SKIPPED test-markdown-flyspell/yaml-metadata `---- With discount replacing markdown: ,---- | Ran 543 tests, 540 results as expected, 0 unexpected, 3 skipped (2024-06-11 04:55:59+0000, 1.410098 sec) | 1 expected failures | | 3 skipped results: | SKIPPED test-markdown-flyspell/check-word-p | SKIPPED test-markdown-flyspell/remove-overlay | SKIPPED test-markdown-flyspell/yaml-metadata `---- So regardless of dropping markdown or adding discount, the test behavior did not change. I think now with discount replacing markdown it is in a better state. Thanks, and PTAL. [1] https://salsa.debian.org/emacsen-team/dh-elpa/-/blob/master/dh_elpa#L44 [2] https://salsa.debian.org/emacsen-team/markdown-mode/-/commit/d900ef55bb0ecfc91609355ad6dd4cad2a578f4d [3] https://salsa.debian.org/emacsen-team/dh-elpa/-/blob/master/elpa.pm?blame=1 [4] https://salsa.debian.org/emacsen-team/markdown-mode/-/blob/master/markdown-mode.el?ref_type=heads#L97 [5] https://salsa.debian.org/emacsen-team/markdown-mode/-/commit/10deb2fb6c00fb83a96226fb0c7645af0672d676 [6] https://salsa.debian.org/emacsen-team/markdown-mode/-/commit/adfe33a78fac11877c32fe2f5c87284b0c1d6463 -- Xiyue Deng
signature.asc
Description: PGP signature
