Xiyue Deng <manp...@gmail.com> writes: > [[PGP Signed Part:Undecided]] > 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.
Friendly ping. -- Xiyue Deng
