Hi Nicholas, Nicholas D Steeves <s...@debian.org> writes:
> Xiyue Deng <manp...@gmail.com> writes: > > Git pull, then read along with the reply that follows inline. > >>> Xiyue Deng <manp...@gmail.com> writes: >>> >>>> * 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. > > Some team members voluntarily accommodate backports and/or > oldstable-backports. Write something like "No longer needed" or > something to that effect, if it's buildable on bookworm and bullseye. > If not buildable, talk to whoever added the B-D. > > If "Drop unnecessary Build-Depends on markdown" were true than discount > shouldn't become a B-D. So there's a major contradiction in your > changelog entries and/or thinking. Likewise, in the interest of > timeliness I'm fixing this myself. Read the updated changelog carefully > and recursively follow any references. Also, consider that if markdown > wasn't a B-D we wouldn't have received this RC bug notifying us of > markdowns removal, and it's probable no one would have noticed until > users complained. That might have been after Trixie's release. If > nothing else, the B-D is worth it for that. > (Here your response was below a past discussion on dh_elpa_test but was really about B-D, and I got a bit confused, but I'll assume the previously quoted texts are not relevant here. I believe your were referring to a later part that discusses B-D.) I'd like to clarify that the contradiction in the changelog was an oversight and I tried to fix that: I realized that `discount' provides a compatible markdown command and should be a suitable replacement of the `markdown' and used that in place of markdown. I tried to remove that entry about dropping, as you can see from my first upload on mentors[1]. It got overwritten due to a later `gbp dch` as I didn't commit that changelog yet and added more work and hence that change was lost. So in retrospect, I realized the early change mentioning that `markdown' is "unnecessary" was incorrect and tried to replace it with `discount', but forgot to remove the earlier entry generated from `gbp dch`. Please also see my previous reply (quoted below) for my analysis on how markdown-mode checks for available commands to process. I guess the better word to describe it is "optional" instead of "unused" indeed, which I tried to remove from d/changelog anyway. > Why should your colleagues give you the time of day (ie: spend any of > their time to review your work) if you systematically refuse to trust > their work? Doing security audits would be a better use > skepticism/suspicion ;) > I hope the previous explanation would clarify that there is no ill-intent to discredit any of the existing work. >>>> * 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? > > For upstream, we copy what upstream says, and it's non-authoritative. > For debian/*, we are the authority, and it affects all of our > downstreams. This means one needs to get it right. > > Rather than communicating on the level of symbol manipulation, I expect > you to write in a way that demonstrates a minimal understanding of > copyright and license. Go to #debian-mentors on IRC and ask for reading > material, then get back to me about what your proposed change actual > means. Key terms: "introduction to copyright free software dfsg Debian > packaging maintainers". > > You did add yourself to copyright, which is fine, but just remember that > I expect that your work will evolve from symbol manipulation to > understanding. I've added a TODO for you in the changelog. Answer > those questions, write a human-authored response rather than > machine-talk regurgitation of the git data and I think you'll meet the > minimum requirements. > I hoped my previous explanation could clarify that I have no ill-intention. I guess I'll have to be more explicit here: I think copyright is used to acknowledge intellectual work and to ensure that Debian has legal permission to host and redistribute the work, and making it right is important. I believe my current approach to add all people previously worked on `debian/*' is on the right track. I'll address the TODOs in later part. >>>> 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]. > > Follow the breadcrumb trail that I made more explicit for you. > >> 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. > > Really? It looks to me like you proved this: > > (setq markdown-state 3) > (setq without-markdown-state 3) > (setq with-discount-state 3) > (setq markdown-to-discount-diff 0) > (setq markdown-state (/ markdown-state markdown-to-discount-diff)) > > I know this isn't idiomatically correct, but I want non-LISPers to be > able to understand this gentle point. > Sorry but you completely lost me here, especially the calculation in this last assignment. What does it mean? > Please resolve this issue by completing the TODO item I put in the > changelog. > For completeness let me copy your TODO items here: ,---- | TODO: read the source code to see if it needs to be patched to use | discount-by-default during tests, as well as in the default config. That | work will go in this section. Answer if they're unit tests or functional | tests? That justification needs to go here. If I have to look this | stuff up myself then you can't share credit. `---- The tests need not to be patched. Looking into tests/markdown-test.el, and try to search for `markdown-command', you can find * 11 occurrences that it's bound to `markdown-command-identity' that copies a given region, which is basically a mock implementation; * 1 occurrence that it's bound to a lisp function that records all arguments; * 3 occurrences that it's bound to a `pandoc' command with or without arguments; - This shows why pandoc is not used as an alternative in B-D because there are tests directly use it. * 1 occurrence that it's bound to a non-existent command; * 1 occurrence it's bound to `false'; now, this leaves 1 occurrence that it actually checks the availability of the command using `executable-find'. As I mentioned in previous mail, it will try to find the first available command in `markdown', `pandoc', or `markdown_py'. As we have package "discount" replacing package "markdown" which also provides `/usr/bin/markdown', it should still work. In fact, the package will still work with pandoc as the only B-D because it can handle markdown format just fine. This also explains why the 3 scenarios produce the same result: as long as the command "markdown" or "pandoc" are available, the tests will work as intended. Regarding your question of test types, except the 4 tests that use a real markdown-command which are functional tests, all others can be considered unit tests with a mock markdown-command. ,---- | P.S: Were we to "discount | libtext-multimarkdown-perl", the latter would | never be tested, because the first branch is always chosen. `---- Alternatives in B-D are useful in case one package is not available in all Debian supported architectures so that the rest of the alternatives can be used instead. In this case, yes the latter would never be used because discount is available in all supported archs. > Best, > Nicholas > [1] Upload #1 on https://mentors.debian.net/package/markdown-mode/ P.S. I feel that your wording in the last email is quite strong, and I don't think this was necessary. If there is something I did or said that you dislike or make you uncomfortable please let me know. -- Xiyue Deng
signature.asc
Description: PGP signature
