package-review February 2007

package-review@lists.fedoraproject.org

1 participants
2523 discussions

fedora-review granted: [Bug 226669] Merge Review: zip
by Red Hat Bugzilla 07 Feb '07

07 Feb '07

Bug 226669: Merge Review: zip Product: Fedora Extras Version: devel Component: Package Review Ruben Kerkhof <ruben(a)rubenkerkhof.com> has granted Ivana Varekova <varekova(a)redhat.com>'s request for fedora-review: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226669 ------- Additional Comments from Ruben Kerkhof <ruben(a)rubenkerkhof.com> I don't think the file BUGS is very relevant and MANUAL is a duplicate of the manpage your already installing. Please consider removing those. I don't see any further blockers, so this package is approved.

1 0

fedora-review granted: [Bug 225641] Merge Review: chkconfig
by Red Hat Bugzilla 07 Feb '07

07 Feb '07

Bug 225641: Merge Review: chkconfig Product: Fedora Extras Version: devel Component: Package Review Ruben Kerkhof <ruben(a)rubenkerkhof.com> has granted Ruben Kerkhof <ruben(a)rubenkerkhof.com>'s request for fedora-review: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225641 ------- Additional Comments from Ruben Kerkhof <ruben(a)rubenkerkhof.com> Thanks. I don't see any blockers, so this package is approved.

1 0

fedora-review requested: [Bug 225207] Review Request: libsmbios - library for userspace smbios table parsing
by Red Hat Bugzilla 07 Feb '07

07 Feb '07

Bug 225207: Review Request: libsmbios - library for userspace smbios table parsing Product: Fedora Extras Version: devel Component: Package Review Matt Domsch <Matt_Domsch(a)dell.com> has asked for fedora-review: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225207

1 0

fedora-review denied: [Bug 225963] Merge Review: kdelibs
by Red Hat Bugzilla 07 Feb '07

07 Feb '07

Bug 225963: Merge Review: kdelibs Product: Fedora Extras Version: devel Component: Package Review Rex Dieter <rdieter(a)math.unl.edu> has denied Rex Dieter <rdieter(a)math.unl.edu>'s request for fedora-review: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225963 ------- Additional Comments from Rex Dieter <rdieter(a)math.unl.edu> On first glance (otherwise, looking good): * SHOULD omit Requires: cups ... Obsoletes: %{name}2 Obsoletes: kdesupport Obsoletes: kdoc that's all ancient history, no need, imo, to carry the baggage anymore. * -devel pkg SHOULD Requires: qt-devel unversioned here is ok, main pkg already includes Requires: qt >= EVR omit legacy crud: Obsoletes: kdesupport-devel * SHOULD omit Requires(pre,post): desktop-file-utils per http://fedoraproject.org/wiki/Packaging/ScriptletSnippets it is no longer required (FC-5+)

1 0

fedora-review granted: [Bug 226411] Merge Review: setserial
by Red Hat Bugzilla 07 Feb '07

07 Feb '07

Bug 226411: Merge Review: setserial Product: Fedora Extras Version: devel Component: Package Review manuel wolfshant <wolfy(a)nobugconsulting.ro> has granted Tim Waugh <twaugh(a)redhat.com>'s request for fedora-review: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226411 ------- Additional Comments from manuel wolfshant <wolfy(a)nobugconsulting.ro> Formal review for release 2.17-20.fc7: - package meets naming guidelines - package meets packaging guidelines - license is GPL (hence OK), matches source; upstream does not include the license, so it isn't included in the package either - spec file legible, in am. english - source matches upstream, last available version, sha1sum 68824494a0b5700f7e999564a59358bf34f79eb1 setserial-2.17.tar.gz - package bilds in mock for devel/x86_64 - no missing BR - no unnecessary BR - no locales - not relocatable - owns all files and directories that it creates, does take not take ownership of foreign files/directories - no duplicate files - permissions ok - %clean ok - macro use consistent - code, not content - no need for -docs - nothing in %doc affects runtime - no static, .pc, .la files - no need for .desktop file - rpmlint is silent on the source; there is one warning for the generated binary: W: setserial spurious-executable-perm /usr/share/doc/setserial-2.17/rc.serial It can be ignored, this is meant as an initscript which ( if needed ) must be installed in /etc/init.d anyway SHOULD - builds cleanly in mock - runs as advertised TODO - upstream should be bugged to included the license in the supplied tar.gz APPROVED

1 0

fedora-review requested: [Bug 225963] Merge Review: kdelibs
by Red Hat Bugzilla 07 Feb '07

07 Feb '07

Bug 225963: Merge Review: kdelibs Product: Fedora Extras Version: devel Component: Package Review Rex Dieter <rdieter(a)math.unl.edu> has asked for fedora-review: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225963

1 0

fedora-review denied: [Bug 226351] Merge Review: qt
by Red Hat Bugzilla 07 Feb '07

07 Feb '07

Bug 226351: Merge Review: qt Product: Fedora Extras Version: devel Component: Package Review Rex Dieter <rdieter(a)math.unl.edu> has denied Rex Dieter <rdieter(a)math.unl.edu>'s request for fedora-review: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226351 ------- Additional Comments from Rex Dieter <rdieter(a)math.unl.edu> Guidelines-wise, looks good, only a few relatively small SHOULD (optional) items to consider: * icons. SHOULD try to follow icon-spec here, and put items in/under %_datadir/icons/hicolor/ somewhere instead of using %_datadir/pixmaps * .desktop files: using namespace/vendor of qt-* *and* using '3' appended to .desktop file names is redundant. Consider removing the '3' from the .desktop file names. * consider changing qtdir from %{_libdir}/qt-3.3 to just %_libdir/qt3. It would then be consistent with qt4 packaging. This is a big change that may induce pain in other packages that currently hard-code the old location, so it may not be worth it. * -devel-docs subpkg SHOULD: 1. should simply be -doc (corrolary: and have no dependency on -devel) 2. assistant should be included here. * SHOULD avoid some of the multilib ickiness, and necessity to use /etc/ld.so.conf.d/qt*, by simply using ./configure -libdir %{_libdir} ie, putting qt's libs into %_libdir directly. qt4 has been using these latter 2 items with success.

1 0

fedora-review requested: [Bug 226351] Merge Review: qt
by Red Hat Bugzilla 07 Feb '07

07 Feb '07

Bug 226351: Merge Review: qt Product: Fedora Extras Version: devel Component: Package Review Rex Dieter <rdieter(a)math.unl.edu> has asked for fedora-review: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226351

1 0

fedora-review denied: [Bug 226357] Merge Review: rdate
by Red Hat Bugzilla 07 Feb '07

07 Feb '07

Bug 226357: Merge Review: rdate Product: Fedora Extras Version: devel Component: Package Review Roozbeh Pournader <roozbeh(a)farsiweb.info> has denied Roozbeh Pournader <roozbeh(a)farsiweb.info>'s request for fedora-review: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226357 ------- Additional Comments from Roozbeh Pournader <roozbeh(a)farsiweb.info> Random first notes: * Remove the dot at the end of Summary line. * It seems that there is no upstream. No URL is given, and the Source address doesn't work either. So I can't check that this is the same as the upstream source. (BLOCKER) * Change BuildRoot to %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) * Use the option %{?_smp_mflags} for make. * Don't use %makeinstall, if possible. See http://fedoraproject.org/wiki/Packaging/Guidelines#MakeInstall * Use %defattr(-,root,root,-) instead of %defattr(-,root,root) * Consider using %{?dist} in Release tag. * The binary file 'rdate' is installed with permission 555 in the Makefile. It should be 755. This can be fixed either by patching the Makefile or by explicitly changing the permission in the %install or %files section. (BLOCKER)

1 0

fedora-review denied: [Bug 225794] Merge Review: ghostscript-fonts
by Red Hat Bugzilla 07 Feb '07

07 Feb '07

Bug 225794: Merge Review: ghostscript-fonts Product: Fedora Extras Version: devel Component: Package Review Roozbeh Pournader <roozbeh(a)farsiweb.info> has denied Tim Waugh <twaugh(a)redhat.com>'s request for fedora-review: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225794 ------- Additional Comments from Roozbeh Pournader <roozbeh(a)farsiweb.info> (In reply to comment #2) > 6.0 is an older release (see the timestamp). Worst than that, they are exactly the same thing. The bits are exactly the same! I would still recommend using 6.0, so people won't think we are not using the latest version in Fedora. More random notes: * Your new URL is still not good. The tarball is not provided from the sourceforge servers, and not any real info either. It's a dead project. It just says go to GNU for more info. Use either http://savannah.gnu.org/projects/ghostscript/ or http://www.gnu.org/software/ghostscript/. The second is preferred, as its more user oriented. (BLOCKER) * The URL in the Source line does not work anymore either. Use http://ftp.gnu.org/gnu/ghostscript/gnu-gs-fonts-std-%{version}.tar.gz * You should change the "Requires: fontconfig" line to different lines for requirements after installation and uninstallation. Currently, one can ask rpm/yum to remove both fontconfig and ghostscript-fonts and fontconfig may get removed before ghostscript-fonts, making the post uninstallation scripts fail. A similar scenario can happen with installation. Also, you need to have mkfontscale, mkfontdir, and chkfontpath during some of these. (BLOCKER) Suggested lines: Requires: fontconfig Requires(post): /usr/bin/mkfontscale /usr/bin/mkfontdir Requires(post): /usr/sbin/chkfontpath Requires(post): fontconfig Requires(postun): /usr/sbin/chkfontpath Requires(postun): fontconfig * Copy the files during the %install section using the '-p' option of cp (or use install -p). * Have an empty %build section. * Use %{_datadir} instead of /usr/share in the %files section. Also consider adding a "/" at the end to show that we are actually including a directory and files in there. * There is nothing in the source tarball that says the license of the files is GPL, as the license field of the spec file says. They may as well be proprietary software, as far as a random observer can tell. Since contacting upstream for including a license may not be trivial, the spec file should at least document why we have made sure this is licensed under the GPL. (BLOCKER) * The summary ends with a dot. It shouldn't. * I don't think the use of parenthesized "(TM)" is really necessary in the Summary line. The Fedora EULA already says that all trademarks are owned by their respective owners. * The part of the description that says you'll need to install this for ghostscript to work is not that important to be worth a mention. That is simply a Requires line in the ghostscript package.

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

package-review February 2007