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=653682 --- Comment #5 from Ingvar Hagelund <ingvar@xxxxxxxxx> 2010-11-18 05:21:07 EST --- * Golo Fuchert wrote > Since I am not yet sponsored I can only do an informal review for > this package. Please note that since I am quite new here I may have > overseen some details. Thank you for the effort, Golo, and welcome to Fedora! > jemalloc.x86_64: W: manual-page-warning /usr/share/man/man3/jemalloc.3.gz 65: > warning: `UR' not defined > jemalloc.x86_64: W: manual-page-warning /usr/share/man/man3/jemalloc.3.gz 67: > warning: `UE' not defined > > - honestly, I have no idea what those warnings mean. I saw they were > no blockers in other reviews, but this is a bit > disappointing. Maybe someone can say more about these. These are caused because by som reason troff is unable to parse the .UR (url start) and .UE (url end) tags correctly, though I can't see why this happens. There are a lot of other packages using these tags, where they seem they get parsed correctly. This could be caused by some compatibility strangeness since the source is from the BSD camp. > jemalloc.x86_64: W: no-manual-page-for-binary pprof > - would be nice to have one but no blocker. I removed pprof, since it's part of another package, see comment #3. > [-] Package does not own all directories that are created: > -devel sub-package creates %{_includedir}/jemalloc but doesn't own it Fixed. > [-] -devel package should require the package correctly as %{name} = > %{version}-%{release} (missed the -%{release} part) Fixed. > Further comments: > - The description contains a lot of trademarks. As far as I can tell > they are used properly, but I feel a bit uncomfortable with > that. Don't you think that the users searching for this kind of > package know who uses it for what? However, I _think_ it is safe > like that but maybe others can comment on this. I agree that they are uneccessary. Removed. > - A dot would be the perfect ending to the description of the devel package. Fixed. > - Extensive globbing in the file section is a dangerous thing; you > might include files and dirs you don't want to include and it gets > more legible if you are a bit more explicit. Especially when you > glob just one file like in > %{_mandir}/man3/*. Fixed. > - Would do no harm to use the %{name} macro in the SOURCE0 tag. Fixed. * Martin Gieseking > Here are some additional notes: > - the Group of the devel package should be Development/Libraries Fixed. > - I suggest to use the default %description for the devel package: > The %{name}-devel package contains libraries and header files for > developing applications that use %{name}. Works for me. Fixed. > - the man3 manual page belongs to the devel package as it contains the API > documentation of the library Fixed. * Jussi Lehtola > (In reply to comment #3) > The important thing here is that you should not duplicate files in > interdependent packages. -devel requires the main package, so the > files are going to be present anyway. Fixed. > btw. > Requires: %{name} = %{version} > should read > Requires: %{name} = %{version}-%{release} > unless you have an *extremely good* reason not to use a fully versioned > Requires. Yep, I overlooked that one. Fixed. Thank you too, Martin and Jussi. Updated srpm and specfile at http://users.linpro.no/ingvar/jemalloc Ingvar -- 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