Hi Gianfranco, Thank you for the comments and suggestions, they are really appreciated. Apologies for the delay, I have slowly worked through your email on and off over the past few months. I think I have addressed most of the issues you mentioned (see my comments below), so I have uploaded a fresh version to http://mentors.debian.net/package/w1retap.
Thanks again for reviewing the package, I would appreciate any other comments you have or uploading it if you feel it's ready. On 01 Apr 2016, at 20:52, Gianfranco Costamagna wrote: > control: > "retrieved data" <-- double space > std version is 3.9.7 now > please run wrap-and-sort > please remove quilt from b-d > changelog: > one single changelog entry please > > -rm -f $(CURDIR)/debian/w1retap.1 $(CURDIR)/debian/w1find.1 -> debian/clean > might be easier to maintain > > rules: > dh_auto_install is a no-op please remove All fixed. > dh_makeshlibs --noscripts <-- why? The plugin packages contain .so files, the --noscripts option stops an ldconfig run when installing. Without it I get a lintian useless ldconfig run warnings. > remove quilt Fixed. > libw1retap0-odbc.install <-- is that the mysql wrapper? No, it's a odbc plugin and the mysql plugin is separate. The confusion was caused by a typo on my part. In anycase, both are now fixed. > service file: > ExecStop=/bin/kill $MAINPID > > really needed? Fixed. > patches: remove-data-time-macro.patch > you can dpkg-parsechangelog and export the changelog date an build date. > (this will make the package reproducible I think) The current upstream code (src/w1retap.c) uses the "__DATE__" and "__TIME__" GCC standard macros directly. I don't think dpkg-parsechangelog can affect these macros. My patch replaces these macros with __BUILD_DATE__. > question: > usually libraries are ship in this way > > libfooSONAME shipping libfoo.so.SONAME > > libfoo-dev with an hard dependency on libfooSONAME > and a libfoo.so symlink to libfoo.so.SONAME and /usr/include/foo > headers files. > > In your package you are shipping them together > > not an issue, but do you know what you are doing here? > are them useful and being linked outside the package? > are them just internal? The .so files are not useful or used and outside the w1retap, they are all internal. Thus I agree and have removed all the libw1retap* packages. They act more like plugins so I now have a main w1retap package and 5 separate plugin package for each database type. > lots of stuff (systemd script udev rules) should be upstreamed to me There are a number of patches that I will send upstream: - add-etc-w1retap-conf.patch - movetmp.patch - remove-data-time-macro.patch - udev rule - systemd service > check-all-the-things review: > codespell --quiet-level=3 > cppcheck -j1 --quiet -f . > grep -riE 'fixme|todo|hack|xxx' . > grep -r '/tmp/' . I did run all of the above and found some issues. Some of the issues seemed to be more related to actual upstream development rather than packaging. Most notably predicable file creation in /tmp, which is fixed in the above mentioned movetmp.patch. Did you spot anything else that needing fixing? Regards -- Tom