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=740160 --- Comment #4 from Craig Barnes <cbgnome@xxxxxxxxx> 2011-09-25 16:56:34 EDT --- (In reply to comment #3) > Hi! Someone else will need to sponsor you, but here's some comments: > > - You should always upload a SRPM file somewhere when updating the spec file > and post a link here. It will make it easier for reviewers. Once you upload > one, I'll take a closer look. Sorry I forgot to make the SRPM available when I bumped the spec. I will keep SRPMs here from now: http://dl.dropbox.com/u/2682668/discount-2.1.1.3-4.fc15.src.rpm > - You seem to have the subpackage idea right: discount will be the command line > utility, libmarkdown the library itself and libmarkdown-devel will contain the > headers. However, you need to fix the following: > > 1. Explicit requirements between the packages. Since the command line utility > depends on the libmarkdown library, you should add an explicit require on the > base package for libmarkdown. (Following the syntax here: > http://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package) I did this and rpmlint gave an error (E: explicit-lib-dependency). I researched it a little and it seems that if the base package depends on a sub-package library, RPM handles the dependency automatically. Correct me if I'm wrong. I have left this one out for now. > 2. In addition, if a user wants to develop software using libmarkdown, they > will need libmarkdown to use the development files, so add a requirement for > libmarkdown to the devel subpackage too. Fixed. > 3. Non-versioned shared libraries (libmarkdown.so) must be packaged in the > development subpackage > (http://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages). (You > should also remove the noarch parameter from -devel.) Fixed. > - You should select files in the %files section more verbosely: for example: > > 1. For the binary file, just do %{_bindir}/markdown > > 2. For the manual files, do something like %{_mandir}/manx/*.x* (where x is the > number) > > 3. For the library files, you can select the versioned files with > libmarkdown.so.* and the non-versioned file with just libmarkdown.so. Fixed (I think). Did everything you suggested here but more feedback appreciated. > - Needless to say, the description and summary for libmarkdown should contain > something. Done. > - If you don't plan on building this package for EPEL5, you can remove %clean. > Otherwise, you need to add a BuildRoot tag and do rm -rf %{buildroot} in the > beginning of %install too. > (http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#BuildRoot_tag) Yeah, I had just blindly copied that part, not really understanding what it was for. I've moved the -rf %{buildroot} back to the top of install. Hopefully that's correct. feedback appreciated. Thanks a lot for the feedback. It helped to clear up a few confusions I had and to fix most of the rpmlint warnings. There's just 1 remaining warning about libmarkdown-devel not having any docs. Also, I have just noticed that the old description about "developer-oriented man pages" is no longer accurate. Will fix and bump shortly. -- 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