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=525786 --- Comment #3 from Naoki IIMURA <naoki@xxxxxxxxxxxxxx> 2009-09-28 07:59:51 EDT --- Thanks for comments. > * License statement of the spec file itself > - It is not forbidden, however ususally spec files in Fedora > packages don't contain license statements on the spec files > themselves. OK. I've removed the license statements. > * Naming/EVR (Epoch-Version-Release) > - The release number of the spec file should have integer > value except for the cases written in > https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Package_Release > > - And usually adding %{?dist} tag is preferred. > https://fedoraproject.org/wiki/Packaging/DistTag OK. I've update the release number to "2%{?dist}". > * Vendor > - Vendor tag must be removed on Fedora. This tag is automatically imported > on Fedora build server. OK. I've removed the Vendor tag. > * Exclusiveos > - I don't think you have to write this explicitly. OK. I've removed the Exclusiveos tag. > * Perl related packaging treatment > https://fedoraproject.org/wiki/Packaging/Perl > - "Requires: perl >= 5.8.1" does almost nothing because Fedora's perl > rpm has epoch: > <snip> > - And anyway I don't think this is needed because even Fedora 10 > (the oldest branch currently supported on Fedora) uses perl 5.10. OK. I've removed the Requires tag for perl. > - For perl module dependency, please write virtual Provides names > instead of rpm names directly: > https://fedoraproject.org/wiki/Packaging/Perl#Perl_Requires_and_Provides OK. I've updated the perl module dependencies to use virtual Provides names. > * rpmlint issue > ------------------------------------------------------ > popfile.src: W: strange-permission popfile 0775 > popfile.src: W: strange-permission start_popfile.sh 0775 > ------------------------------------------------------ > - All files in srpm should have 0644 permission. OK. I've applied appropriate permissions. > * %setup/%build/%install > - Umm... I think your current spec file is unneededly verbose > and redundant. While listing %files section verbosely can be > accepted, is there any reason you don't prefer to write like > below? (all commentes removed for now) > ------------------------------------------------------ > <snip> Thanks. I've updated the %install section. > Some notes: > - %setup recognizes zip file. "-c" option on setup creates > the diretory beforehand when unpacking source. Also "-q" option > is preferred to suppress messages when unpacking source. I want to pass "-a" option to unzip for converting line endings of the text files. So that I splitted the %setup and %{__unzip}. > - When using "install" or "cp" commands, add "-p" option (or > "-a" for "cp") to keep timestamps on installed files: > https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps Thanks for the information. I've added "-p" option. > - If you want to close macros with {}, please do so consistently > (i.e. please use %{name}) OK. I've done. > * Initscripts treatment > <snip> > Some notes: > - Some Requires(post) or so are missing I've added Requres(post), Requires(preun) and Requires(postun). > - Why your spec file stopps daemon every time this package is > upgraded (on %pre)? > https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Syntax I wanted to stop the daemon before upgrading and restart it after upgrading. Now I've commented out '/sbin/service popfile stop'. > ! Also please check %postun scriptlet example (and the usage of > condrestart) I've written %postun script. > - Please use either "/sbin/service popfile" style or "%{_initrddir}/popfile" > style consistently I've updated the scripts to use "/sbin/service" style. > - Currently using %{_initddir} macro instead of %{_initrddir} is preferred: > https://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscripts_on_the_filesystem I've updated the spec file to use %%{_initddir} macro instead of %%{_initrddir}. > - rpmlint warns: > ----------------------------------------------------------------------------- > popfile.noarch: W: service-default-enabled /etc/rc.d/init.d/popfile > ----------------------------------------------------------------------------- > <snip> OK. I've updated chkconfig line and deleted Default-Start and Default-Stop lines. > * %files > - Now %defattr(-,root,root,-) is preferred on Fedora. OK. I've updated %defattr. > - By the way > ----------------------------------------------------------------------------- > <snip> > ----------------------------------------------------------------------------- > can be unified as > ----------------------------------------------------------------------------- > %files > %{_datadir}/%{name}/Classifier/ > ----------------------------------------------------------------------------- > <snip> Thanks for the information. I've simplified the %files section. I've uploaded the new SPEC and SRPM files: Spec URL: http://getpopfile.org/browser/trunk/linux/fedora/popfile.spec?format=raw SRPM URL: http://getpopfile.org/downloads/popfile-1.1.1-2.fc11.src.rpm Naoki -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review