Bug#747816: Review openalpr

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

Bug#747816: Review openalpr

Tobias Frost-3
Hallo Stefan,

as promised some weeks ago, here's your review :)

Here's an (unordered) list of thing I found during checking your
package. Maybe you want to check them if they are valid and correct
them?

I just saw that the ITP is not owned by you. Did you coordinate with the
one filing the ITP? (If so, you should document it there, if not you
should ask Matt Hill if he intends to to the packaging or not... The
bug's text is not clear here.)

- Upstream released in the meantime 1.1.1 ... Can you update your
package to the latest version?

- The tarball does not match the upstream tarball.
7585916d11fef3ea88bba3249e839524  ../openalpr_1.1.0.orig.tar.gz
d1446343788a52514e78afd33b73fe66  /home/tobi/Downloads/openalpr-1.1.0.tar.gz
(Did you directly pull from the repository -- as there was a .git
directory in it?)

- d/README.source --  I think you don't need this file, let it go... :)

- d/patches/* -- please add a dep3 header; does it make sense to send
the patch upstream?

- d/control -- as you are building a library I miss somehow a -dev
package... Also the package description of the binary package openalpr
needs to explain a little better what this package is for. Currently it
only describes the libary, not the tool(s) you could find in the
package.

- d/copyright is incomplete and should be of the machine-readable
debian/copyright file format (dep5)
(licensecheck and /usr/lib/cdbs/licensecheck2dep5 can help you here, but
you need anyway to check every file..

- any reason not to enable multiarch for the lib?  

- when building you should instruct cmake to be verbose (e.g show the
complete gcc commandline... (I think you just need to "make VERBOSE=1")
in d/rules)

- the package does not build in a pdebuilder enviorment
(chrpath not found; missing B-D?) Is chrpath really needed?

- symbol files for c++ libraries are IMHO a little bit fragile and after
installing chrpath it failed here. So you need to work on those or drop
the symbol file for this moment)

- you manually have a lots of manually added library dependencies on the
binary packge --- are they needed? Shouldn't be shlibs:depends enough
find the needed dependencies?

nitpicks:
- d/* -- I suggest to use wrap-and-sort to sort e.g the dependencies.

- d/control -- there are some trailing white-spaces you should remove
  (wrap-and-sort will do it for you)

- There are many "could avoid a useless dependency" warnings... Can you
check if a --as-needed linker option would work?

Ok, thats it for now. Thanks for working on the package, it's looks like
a quite tricky package to do :)

--
tobi

signature.asc (836 bytes) Download Attachment