Xiyue Deng <manp...@gmail.com> writes: > 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.) >
It turns out that `markdown' has been adopted and is no longer being RMed. So I took the liberty to update the changelog to remove the review question and adding markdown back to Recommends but as last alternative. `discount' is still preferred as B-D. Also, this review has taken over 4 months. As I still haven't received a reply from Nicholas for a while, I would like to invite a wider audience for sponsorship. > -- > Xiyue Deng -- Regards, Xiyue Deng
signature.asc
Description: PGP signature
