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=467958 --- Comment #3 from Hans de Goede <hdegoede@xxxxxxxxxx> 2008-11-04 08:52:21 EDT --- Here is an initial lists with comments and things to fix, note that I've not done a full review yet as I cannot build barry due to the missing .desktop file, see the MUST FIX list. MUST FIX -------- * The tarbal in the SRPM differs from the one provided by upstream (but contents are identical). Did you make any changes to rights or something like that? Please do not do that, please redownload upstream's tarbal and use that as is. * barry.desktop is not included in the srpm, this is because you do not have a Source# line for it, add the following below the Source0 line : Source1: barry.desktop * when you install the .desktop file you refer to it by a path which depends on the build environment, this will for example not work on the buildsys. Always refer to source files using %{SOURCE#}, so instead of: desktop-file-install --vendor="" --dir=%{buildroot}%{_datadir}/applications/ ~/rpmbuild/SOURCES/%{name}.desktop write: desktop-file-install --vendor="" --dir=%{buildroot}%{_datadir}/applications/ ${SOURCE1} * Please remove the following commented lines, they add no information, as the same is already written above them: #%if "(0%{?fc9} || 0%{?fc10})" # $define with_opensync 0 #%else # $define with_opensync 1 #%endif * Drop the (tm) and the "(BlackBerry is a registered trademark of Research in Motion Limited.)) Everywhere, I've consulted out licensing expert on that and that is not necessary (and so ugly) * We don't do soname as packagename in Fedora so please rename the libbarry0 subpackage to barry-libs, so replace: %package -n libbarry0 With: %package libs And do to the same for "%description -n libbarry0" * The Group for the -libs package should be "System Environment/Libraries" * Drop the "Requires: boost", rpm will pick this up automatically" * Rename libbarry-devel to barry-devel, iow replace "%package -n libbarry-devel" with "%package devel" * "Requires: %{name} = %{version}-%{release}" should be "Requires: %{name}-libs = %{version}-%{release}" * Drop the "Conflicts: barry-bcharge", we've never shipped that and using Conflicts is BAD * Drop the "Requires: gtkmm24", rpm will pick this up automatically * Drop the "Requires: libbarry0, libopensync >= 0.22", rpm will pick this up automatically * Drop all the "%attr(0755,root,root)" and the "%attr(0644,root,root)" those should not be necessary * Only include the COPYING file as %doc for the -libs package, the rest will require that so it will always be installed * "[ "%{buildroot}" != "/" ] && %{__rm} -rf %{buildroot}" is deprecated instead just write: "%{__rm} -rf %{buildroot}" * "-p /sbin/ldconfig" must be on the same line as %post[un] so for example: "%post libs -p /sbin/ldconfig" Should fix ---------- * Fix the indentation of the BuildRequires: line (add one space) * Currently your main package is empty. I believe that most users will want the gui tools, so I would like to suggest dropping the gui sub package and put the files currently there in the main package -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review