Red Hat Bugzilla – Bug 276201
Review Request: gt5 - A diff-capable 'du-browser'
Last modified: 2007-11-30 17:12:14 EST
Spec URL: http://timeoff.wsisiz.edu.pl/rpms/gt5/gt5.spec SRPM URL: http://timeoff.wsisiz.edu.pl/rpms/gt5/gt5-1.3d-2.src.rpm Description: Allows to check what takes the most of your hard disk space and track its changes. It's a very small program (in fact a script), but was very useful for me, so I decided to make a package for Fedora.
Ehh, those links weren't to the newest version of a package. Spec URL: http://timeoff.wsisiz.edu.pl/rpms/gt5/gt5.spec SRPM URL: http://timeoff.wsisiz.edu.pl/rpms/gt5/gt5-1.3d-3.src.rpm
Ehh, a package has been waiting for 4 months and in the meantime an author sneakily released a new version ;) Spec URL: http://timeoff.wsisiz.edu.pl/rpms/gt5/gt5.spec SRPM URL: http://timeoff.wsisiz.edu.pl/rpms/gt5/gt5-1.4.0-1.src.rpm
#sed is considered the minimum build environment #see: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions #Requires: sed Sed being in the minumum build environment only affects BuildRequires but not Requires. But for building this packages, sed is not needed, so either you should add Requires: sed to the spec. Also the package requires a console web browser[1] for running, but I do not know how to ensure this with Requires. I will ask on a mailing list. Maybe you should require gzip and coreutils, too, then you should really have all requirements stated explicitly. [1] to be precise: one out of "links links2 elinks lynx w3m" needs to be installed
I thought they can be omitted in both sections. Thanks for point it out. I saw your thread on the mailing list and hopefully some generic provider will be added in some future version of Fedora. I added a note in description about required console web browser. Updated version: Spec URL: http://timeoff.wsisiz.edu.pl/rpms/gt5/gt5.spec SRPM URL: http://timeoff.wsisiz.edu.pl/rpms/gt5/gt5-1.4.0-2.src.rpm
change source URL to http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz you can change %defattr(0644,root,root,0755) to %defattr(-,root,root,-)
(In reply to comment #5) > change source URL to > http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz Why not. I'll do it in the next version. > you can change %defattr(0644,root,root,0755) to %defattr(-,root,root,-) My version is safer IMHO. For example there were problems in (at least) FC6 where due to some coincidences, permissions for files in /usr/share/app/ allowed access for root only (600). I see no cons of my solution.
Who is reviewing this package now?
Mamoru, You proceed with this review. Would like to only CC member for this review.
Well, * For this package it is better that you install gt5 script and man files by yourself using "install -p". This can easily keep timestamps and removes unneeded patch. * For F-8+, text browser provides "text-www-browser" virtial Provides, so for F-8+, this package should require "text-www-browser". * "INSTALL" file is for people who want to "install" this package by themselves and not needed for rpm users.
I made suggested changes, but I'm not really sure that a version with direct use of install command is so much better. Updated files: Spec URL: http://timeoff.wsisiz.edu.pl/rpms/gt5/gt5.spec SRPM URL: http://timeoff.wsisiz.edu.pl/rpms/gt5/gt5-1.4.0-4.src.rpm
Okay. --------------------------------------------------------------------- This package (gt5) is APPROVED by me ---------------------------------------------------------------------
New Package CVS Request ======================= Package Name: gt5 Short Description: A diff-capable 'du-browser' Owners: szpak Branches: FC-6 F-7 F-8 InitialCC: Cvsextras Commits: yes
Imported, built and submitted for testing. Thanks for your help.