Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: bogl - a graphics library and an Unicode terminal emulator https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=214124 colding@xxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |colding@xxxxxxxxx ------- Additional Comments From colding@xxxxxxxxx 2006-11-09 10:01 EST ------- ----------------------------------------------------------- I'm not a member of sponsors so I can only do a pre-review. ----------------------------------------------------------- With that out of the way... >From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines: * rpmlint is not silent. bash-3.1$ rpmlint ./bogl-0.1.18-12.i386.rpm W: bogl no-url-tag bash-3.1$ rpmlint ./bogl-bterm-0.1.18-12.i386.rpm W: bogl-bterm no-url-tag bash-3.1$ rpmlint ./bogl-debuginfo-0.1.18-12.i386.rpm W: bogl-debuginfo no-url-tag bash-3.1$ rpmlint ./bogl-devel-0.1.18-12.i386.rpm W: bogl-devel no-url-tag W: bogl-devel no-documentation bash-3.1$ rpmlint ../../SRPMS/bogl-0.1.18-12.src.rpm W: bogl no-url-tag I can see from the comment in the specthat there really aren't any URL presently, but I think that one should be provided/created. * There is no use of the %find_lang macro in the spec. There are no locale files so maybe this is not needed anyway? * /usr/share/bogl is not owned by any package (use %dir) * /usr/include/bogl is not owned by any package (use %dir) * You are using: "Requires: bogl = %{epoch}:%{version}-%{release}". Why not: "Requires: %{name} = %{version}-%{release}" ? Se also my comment about the epoch tag below. >From http://fedoraproject.org/wiki/Packaging/Guidelines: * Timestamps: Consider using "install -p" and "cp -p". You could use INSTALL="install -c -p" in your make install command Other issues: * I can't see any resaon why you need to use the epoch tag. Version numbers like 0.1.18 are not hard for RPM to parse at all. See e.g. here: http://www.rpm.org/max-rpm-snapshot/s1-rpm-inside-tags.html#S3-RPM-INSIDE-REQUIRES-TAG * There is no COPYING file in the top-level source directory. There should be. * The following files do not have a license notice: - bogl-bgf.c - bogl-bgf.h - bogl-term.h - boxes.h - bterm.ti - mergebdf - README - README.BOGL-bterm - reduce-font.c - utils/add_changelog_line * There is no explicit copyright notice in any of the source files _except_ for the following: - bogl-term.c - bogl-vga16.c - *.bdf * The *.bdf files are not under the GPL, but they appear to be free enough * bogl does not use autoconf/automake. I really do find that the old Makefile way is to inflexible. * Please use "Release: 12%{?dist}" not "Release: 12" * There is a lot of compile warnings. These warning should be reviewd for seriousness. I know this is just me being overly strict, but I would prefer the Werror compile flag to be mandatory for all F[C,E] packages. HTH, jules -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review