Re: Review of python-bonsai
Hi Louis-Philippe, I think I have addressed all your comments. I have pushed fixes in a temporary branch. I'll integrate them on debian/master once the review is finished: https://salsa.debian.org/python-team/packages/python-bonsai/-/commits/review/ See my replies below: Louis-Philippe Véronneau, Dec 13, 2021 at 23:07: > * I'm curious to why you need to set "export LC_ALL = C.UTF-8". This was a leftover from another package. It is useless. > 5. d/tests: > > I don't have an autopkgtests setup that has machine-level isolation. You > ran that code and it works? Yes, here is the build log: http://files.diabeteman.com/juoghahf0teG3Phi/python-bonsai_1.3.0-1_amd64.build.txt I tried to reproduce the test environment which is prepared for the github CI actions. This involves generating a docker container with an LDAP server installed and configured, running that container with elevated privileges and run the test suite to talk to that container. It also does some tc voodoo to force timeouts on the TCP connections (see .ci/delay.py). I did not manage to run autopkgtests with containers but since the test needs to run multiple services, a virtual machine made more sense to me. Also, the use of tc may be a bit too much for a container (I'm not sure about the limitations, depending on the host). Some tests are explicitly disabled because I could not get them to work. I will ping the upstream developer with this when I get a chance. > 7. Upstream code > > Have you read the upstream code? It's something you should do (and you > should read all the changes for each new update). Not that you have to > do a proper security audit, but you should go through the code to be > sure there's no obvious or dangerous things in there. I have read most of the python code and it looks well written, documented and tested. C python extensions code is always rather obscure but I did not see anything suspicious. I am no expert in libldap2 however. The project has been around for some years now. It looks stable enough to go in Debian in my opinion. Thanks a lot for your time!
Review of python-bonsai
Hi! Here's my review of python-bonsai: 1. d/control: * You don't need the "(>= 3.6)" restriction for python3-all-dev, as that version isn't even in oldstable. * sphinx-common isn't needed for the source package, as python3-sphinx depends on it * "python3 (>= 3.6)" is not required for python3-bonsai, ${python3:Depends} should take care of that for you. * IMO, python3-bonsai should recommend or suggest python3-bonsai-doc, but that's up to you. - 2. d/copyright: * you forgot to add a debian/* section. AFAIU, noirello isn't the one who wrote d/rules :) * .appveyor/run_with_env.cmd is licensed CC0. You probably don't need those files, so you can exclude them from the source package using Files-Excluded in d/copyright * the MIT license in Debian is named "Expat", for historical reasons. - 4. d/rules: * You left "export DH_VERBOSE = 1" uncommented. * I'm curious to why you need to set "export LC_ALL = C.UTF-8". - 5. d/tests: I don't have an autopkgtests setup that has machine-level isolation. You ran that code and it works? - 6. d/watch: You left "" in there instead of replacing it by the actual project's name (have a look at Lintian) :) Note you can use the "git tag" mode to simplify this file (not that it's required, your file works as-is): [1] - 7. Upstream code Have you read the upstream code? It's something you should do (and you should read all the changes for each new update). Not that you have to do a proper security audit, but you should go through the code to be sure there's no obvious or dangerous things in there. Otherwise, good job! Fix those, ping me and if it's OK, I'll read the upstream code myself and sponsor it. Cheers, [1]: https://salsa.debian.org/python-team/packages/python-mediafile/-/blob/debian/master/debian/watch -- ⢀⣴⠾⠻⢶⣦⠀ ⣾⠁⢠⠒⠀⣿⡁ Louis-Philippe Véronneau ⢿⡄⠘⠷⠚⠋ po...@debian.org / veronneau.org ⠈⠳⣄