On Wed, Apr 12, 2017 at 3:10 PM, Philippe Thierry wrote: > https://mentors.debian.net/debian/pool/main/o/openscap-daemon/openscap-daemon_0.1.6-1.dsc
I don't intend to sponsor this, but here is a quick review: Please encourage upstream to port the project to Python 3, we are getting closer to the point where Python 2 may be unsupported in some distributions. The Debian systemd service file has an incompatible name with upstream but is identical. Please remove the Debian copy and just use the upstream systemd service file. If you want to add the full name upstream, IIRC systemd supports service aliases so you could talk upstream into adding an alias. There are typos in debian/README.Debian, please use a spell checker on it. I think the sysvinit-start-stop-daemon test should use the service script instead of directly running the init scripts. I don't think mentioning DEP-3 in patch headers is useful, thanks for following DEP-3 though. Please try to get all the patches included upstream or fix upstream so they are not needed. ISTR sysvinit supports initially-disabled services, so I think you should drop OSCAPD_START and the /etc/default/openscap-daemon file and use that instead if possible. The short Description is slightly awkward, I'd suggest running it by the debian-l10n-english list. Typos in debian/control: s/that allow/that allows/ s/check/checks/ s/associated to/associated with/ <missing period at the end> I would suggest replacing the specific product name 'Docker' in debian/control with the generic word 'container', unless it doesn't support checking other container types? You might want to strip the installation instructions from the upstream README.md file before installing that into the binary package, since users of the binary packages do not need install instructions. debian/openscap-daemon-docs.docs mentions a README.source file but I can't see that anywhere in the package. I'd suggest running this command. It will make diffs of the debian/ dir easier to view: wrap-and-sort --short-indent --wrap-always --sort-binary-packages --trailing-comma Please add some upstream metadata: https://wiki.debian.org/UpstreamMetadata Some of the below automated checks are upstream issues, please talk to upstream about automatically running these tools in their CI systems. Automated checks: build: E: dh_python2 dh_python2:408: no package to act on (python-foo or one with ${python:Depends} in Depends) adequate: openscap-daemon: py-file-not-bytecompiled /usr/lib/python2.7/dist-packages/openscap_daemon/__init__.py <lots more> lintian: P: openscap-daemon source: debian-watch-may-check-gpg-signature check-all-the-things: $ find . -type f -iname '*.sh' -exec checkbashisms {} + <lots> $ env PERL5OPT=-m-lib=. cme check dpkg Warning in 'control source Build-Depends:2' value 'python (>> 2.7)': unnecessary versioned dependency: python (>> 2.7). Debian has oldstable -> 2.7.3-4+deb7u1; stable -> 2.7.9-1; testing -> 2.7.13-2; unstable -> 2.7.13-2; Warning in 'control binary:"openscap-daemon" Depends:2' value 'python (>> 2.7)': unnecessary versioned dependency: python (>> 2.7). Debian has oldstable -> 2.7.3-4+deb7u1; stable -> 2.7.9-1; testing -> 2.7.13-2; unstable -> 2.7.13-2; Warning in 'source options tar-ignore' value '.git|.*.pyc': There's a missing element in Dpkg::Source::Option. Please log a bug against libconfig-model-dpkg-perl mentioning the missing element and its relevant documentation. $ codespell --quiet-level=3 <lots> $ debmake -k <says some files are LGPL not GPL> $ fdupes -q -r . | grep -vE '/(\.(git|svn|bzr|hg|sgdrawer|pc)|_(darcs|FOSSIL_)|CVS)(/|$)' | cat -s ./atomic-git/f24_spc/Dockerfile ./atomic-git/f23_spc/Dockerfile ./oscapd.service ./debian/openscap-daemon.service ./atomic/f24_spc/install.sh ./atomic/rhel7_spc/install.sh ./atomic/centos7_spc/install.sh ./atomic/f23_spc/install.sh ./atomic/f24_spc/config.ini ./atomic/rhel7_spc/config.ini ./atomic/f23_spc/config.ini # check if these can be switched to https:// $ grep -rF http: . <lots> $ env PERL5OPT=-m-lib=. license-reconcile <says some files are LGPL not GPL, says there are copyright mismatches> # These command check style. While a consistent style # is a good idea, people who have different style # preferences will want to ignore some of the output. # Do not bother adding non-upstreamable patches for these. $ find . -type f -iname '*.py' -exec pep8 --ignore W191 {} + $ proselint . $ pydocstyle . <lots> $ find . -type f -iname '*.py' -exec pyflakes {} + ./openscap_daemon/cli_helpers.py:54: local variable 'total_width' is assigned to but never used ./openscap_daemon/compat.py:46: redefinition of unused 'subprocess_check_output' from line 23 $ find . -type f -iname '*.py' -exec pyflakes3 {} + ./openscap_daemon/cli_helpers.py:27: undefined name 'raw_input' ./openscap_daemon/cli_helpers.py:54: local variable 'total_width' is assigned to but never used ./openscap_daemon/compat.py:46: redefinition of unused 'subprocess_check_output' from line 23 ./debian/lib/release.py:33:34: invalid syntax print 'Loading ReleaseNotes from ' + BASEURL + VERSION $ find . -type f -iname '*.py' -exec pylint --rcfile=/dev/null --msg-template='{path}:{line}:{column}: [{category}:{symbol}] {obj}: {msg}' --reports=n {} + $ find . -type f -iname '*.py' -exec pylint3 --rcfile=/dev/null --msg-template='{path}:{line}:{column}: [{category}:{symbol}] {obj}: {msg}' --reports=n {} + <lots> $ find . -type f \( -iname '*.sh' -o -iname '*.bash' -o -iname '*.zsh' \) -exec shellcheck {} + <lots> $ find . -type d \( -iname .bzr -o -iname .git -o -iname .hg -o -iname .svn -o -iname CVS -o -iname RCS -o -iname SCCS -o -iname _MTN -o -iname _darcs -o -iname .pc -o -iname .cabal-sandbox -o -iname .cdv -o -iname .metadata -o -iname CMakeFiles -o -iname _build -o -iname _sgbak -o -iname autom4te.cache -o -iname blib -o -iname cover_db -o -iname node_modules -o -iname '~.dep' -o -iname '~.dot' -o -iname '~.nib' -o -iname '~.plst' \) -prune -o -type f ! \( -iname '*.bak' -o -iname '*.swp' -o -iname '#.*' -o -iname '#*#' -o -iname 'core.*' -o -iname '*~' -o -iname '*.gif' -o -iname '*.jpg' -o -iname '*.jpeg' -o -iname '*.png' -o -iname '*.min.js' -o -iname '*.js.map' -o -iname '*.js.min' -o -iname '*.min.css' -o -iname '*.css.map' -o -iname '*.css.min' -o -iname '*.wav' \) -exec env PERL5OPT=-m-lib=. spellintian --picky {} + <lots> $ vulture . <lots> $ cats -c python2-bandit <lots> -- bye, pabs https://wiki.debian.org/PaulWise
