Re: FreeMedForms 0.8.2 New upstream
Hi Eric,
On Fri, Mar 08, 2013 at 12:06:54PM +0100, Eric Maeker wrote:
> Oops, I just forget to commit ;)
> So yes it is the 0.8.2.1-1 and I corrected the date too.
>
> Thanks for your pertinent review !
After beeing sure to work on your latest status I did a more thorough
review and I now have additional remarks. These remarks were triggered
by lintian errors which were not properly covered by overrides.
Previously they were written down with explicit versioning which made
them ineffective for any new upstream release. I now used wildcards
which are working better. It would be a good idea if you would fire
up lintian before you offer the package for sponsering. :-)
When looking at the package I also detected some code duplication
issues:
1. /usr/lib/freemedforms/ and /usr/lib/freeaccount both have
Account.pluginspec
AccountBase.pluginspec
Core.pluginspec
DataPackPlugin.pluginspec
libAccount.so
libAccountBase.so
libCore.so
libDataPackPlugin.so
...
(and several others.) IMHO this should be strictly avoided.
The proper way to do this is either move these common files
into the package freemedforms-libs or to create a package
libmedforms<version>_<debversion>.deb
package just containing these libs in a dir
/usr/lib/freemedformslibs
(or some more reasonable dir like freemedforms-common) and make
both packages depend from this lib package. This potentially even
might solve the binary-or-shlib-defines-rpath issue once and for
all. In any case the code duplication needs to be avoided.
In my commit that fixes the lintian overrides I intentionally
left
freeaccount: binary-or-shlib-defines-rpath usr/lib/freeaccount/libIdentity.so /usr/lib/freemedforms-common
untouched to raise some signal.
2. Another code duplication is
freemedforms-libs: binary-or-shlib-defines-rpath usr/lib/freemedforms-common/libquazip.so.1.0.0 /usr/lib/freemedforms
Didn't you just packaged libquazip? So why not linking against
this package rather than injecting another rpath issue?
Kind regards
Andreas.
--
http://fam-tille.de
Reply to: