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=678634 --- Comment #2 from Hedayat Vatankhah <hedayatv@xxxxxxxxx> 2011-03-13 19:09:27 EDT --- (In reply to comment #1) > Hello Hedayat, Hi! Thanks a lot for reviewing this package :) > > Few things to note : > > - Consider using lower case for the spec and package name : > > http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Case_Sensitivity When I was selecting the name I checked various placing in the code and the project and I conclude that upstream prefers Saaghar to saaghar, so I decided to go with the former (e.g. the archive file name and the name of Debian packages generated by upstream). > > - BuildRoot and %clean are not required any more : > > http://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag OK, removed. > > - Add some description for the patches - are they Fedora specific? Can the > patches be integrated into upstream? See : > > http://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment > Oops, sorry for forgetting that. Added. > - Use desktop-file-validate : > http://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usage Added (was a little misinterpretation :P). > > - Looks like a typo in the release field : > > Release: 1.1051%{?dist} Nope. A little description added to the first patch. This is in fact a minor version used by upstream for the version with Saaghar-upstream_update.patch patch applied (the version is directly extracted from a version macro in source files). > > - You can omit the %dir in the files section : > > %dir %{_datadir}/saaghar I don't want to add the whole directory to the main package. There is a DB inside %{_datadir}/saaghar which is included in Saaghar-data sub-package. But I wanted the main package to own the directory. > > > HTH Thanks again. Updated SRPM and SPEC files are here: SPEC: http://hedayat.fedorapeople.org/reviews/Saaghar/0.7.2-2/Saaghar.spec SRPM: http://hedayat.fedorapeople.org/reviews/Saaghar/0.7.2-2/Saaghar-0.7.2-2.1051.fc14.src.rpm -- 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