Control: retitle -1 RFS: markdown-mode/2.6-3 [Team] -- mode for editing Markdown-formatted text files in GNU Emacs
Xiyue Deng <manp...@gmail.com> writes: > 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. Friendly ping. (Also updated the package version in title w.r.t. the recent rebuilt against dh-elpa.) -- Xiyue Deng
