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(a)gmail.com> 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).
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_hav...
Oops, sorry for forgetting that. Added.
- 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.1...
--
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.