poverobuosodonati <poverobuosodon...@gmail.com> writes: >> 1. Let's call the file a part of Emacs (we plan to move tests to Emacs >> eventually) > > For this I would need some additional support as I don't exactly > understand what you mean here. Do you mean where to put it as in like > "emacs/test/lisp" instead of "org-mode/testing/lisp"?
No. I was mostly objecting "this file is not a part of Emacs" line. Better use testing/lisp/text-ox-md.el header that does not have such line. (This thing is rather minor) >> 2. Please prefix the macro and function names (with-...) > Done. Sorry, I was not clear. I asked you to use a unique prefix, not just with-... prefix. Something like (test-ob-csharp-with-... ...) See D.1 Emacs Lisp Coding Conventions section of Elisp manual. Also, "with-" specifically is mostly used for macros. I did not mean to rename functions (it makes no sense there). Finally, `with-try-set-dotnet-version' is questionable. It is used once so you may as well put the code directly inside `with-newest-dotnet'. Here is what I suggest: 1. Rename `with-find-dotnet-version' to `test-ob-csharp--find-dotnet-version' 2. Rename `system-dotnet-version' to `test-ob-csharp-system-dotnet-version' 3. Get rid of `with-try-set-dotnet-version' inlining its code directly into `with-newest-dotnet' 4. Rename `with-newest-dotnet' to `test-ob-csharp-with-newest-dotnet' 5. `test-ob-csharp-with-newest-dotnet' may benefit from (declare (indent 1)) Also, few more comments on the documentation + tests: 1. There is no need to document :dir header argument. It is already described in the main Org manual. 2. :main yes does not appear to be test-covered unless I miss something 3. :nugetconfig is not test-covered 4. :framework is not test-covered 5. :class is not test-covered (when set to a string) -- Ihor Radchenko // yantar92, Org mode maintainer, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92>
