Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=712624 Richard Shaw <hobbes1069@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hobbes1069@xxxxxxxxx --- Comment #1 from Richard Shaw <hobbes1069@xxxxxxxxx> 2011-06-27 14:59:26 EDT --- If you don't need a sponsor then I can review your package. Here's a quick and dirty review of the spec file. I'll add my comments inline with OK stuff snipped out: # Note: devel package does not contain any headers. Do I need to add them # .so files are shared libraries, I need to call /sbin/ldconfig, but where? In a # post section for the devel package? # Schemas handling also needs to be looked at. I'm not sure if you need a devel package. Is there any reason a program would want to build against this one? If so, then it probably should contain the headers, if not, then probably no. I don't think you need to call ldconfig since you're putting the libraries in a named directory and not in the default (i.e. /usr/lib{,64}). I'm not qualified to review the schema portion so we'll need some help on that part. # applied all the patches from the debian package # svn export svn://svn.debian.org/svn/debian-med/trunk/packages/aeskulap/trunk/ aeskulap-debian Patch0: %{name}-circular-svg.patch Patch2: %{name}-DcmElement.patch Patch3: %{name}-desktop.patch Patch4: %{name}-findAndCopyElement.patch Patch5: %{name}-gcc.patch Patch6: %{name}-i18n.patch # This is used to update the configure.in before running autoreconf fo update the autotoolization. fo -> to # Not used in spec file. It's listed here so it's there with the sources Patch7: %{name}-configure.patch Patch8: %{name}-oflog.patch Patch9: %{name}-patientNames.patch # applied patch7, ran autreconf -if, and then took a diff to generate this patch # The "-if" flag is to update the libtool macros. Patch10: %{name}-autotools.patch I wonder if this is the best way to handle this. I've run into a problem like this with a package I maintain on RPM Fusion. I ended up calling autoconf from %build, i.e.: """ %build # Necessary due to patched configure.in /bin/bash ./autogen.sh """ My way is pretty ugly too. Not sure which way is better. BuildRequires: dcmtk-devel BuildRequires: gettext intltool libpng-devel libjpeg-turbo-devel BuildRequires: libtiff-devel gtkmm24-devel libglademm24-devel BuildRequires: gconfmm26-devel libtool BuildRequires: openssl-devel BuildRequires: gettext-devel BuildRequires: tcp_wrappers-devel BuildRequires: desktop-file-utils BuildRequires: GConf2 Requires(pre): GConf2 Requires(post): GConf2 Requires(preun): GConf2 I couldn't find anything definitive on this so hopefully someone more knowledgeable will chime in. Since you are already requiring "gettext-devel", I'm not sure you need "gettext". Also, since GConf2 is a BR: I don't think the BR(pre,post,preun)'s are needed or even do anything. rm -rvf dcmtk This line should have a comment above it. I assume you're removing a bundled library? touch ./NEWS If there's no NEWS file upstream I don't think adding an empty one is necessary, in fact I believe rpmlint will complain. # remove .la files. Is this sufficient? find $RPM_BUILD_ROOT -name "*.la" -exec rm -fv '{}' \; Yup, that will do it. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review