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=758166 Tom "spot" Callaway <tcallawa@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tcallawa@xxxxxxxxxx --- Comment #2 from Tom "spot" Callaway <tcallawa@xxxxxxxxxx> 2011-12-02 13:42:36 EST --- Okay. There are quite a few things to work on here: * Please start your release counter at 1, not 0 (unless it is some sort of pre-release package as documented in the guidelines) * There is no need to have this section at all: # -- package information %define _name thrift %define _version 0.6.1 %define _release 1 %define _group Development/Libraries # -- filesystem information %define _prefix /usr # -- debug package Just populate the Name:, Version:, Release:, Group: fields, as those will then automatically define %{name}, %{version}, %{release}, %{group}, respectively. You also should never need to override _prefix. * The BuildRoot is not one of the approved choices, see: https://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#Distribution_specific_guidelines * Don't use 0%{?rhel_version} for conditionals. Use 0%{?rhel} (as described in https://fedoraproject.org/wiki/Packaging:DistTag) * Explicit Requires should be for foo = %{version}-%{release}, not just foo = %{version}. * The Group tag would not be identical for all subpackages. Not that it matters very much, to be honest, that tag is not required in RHEL 6 or newer, but if you're going to do it, please be more accurate. * Also, the %description would not be identical for all subpackages. Please make them correct and applicable for each subpackage entry. * Please don't double-up config options to configure on a single line. It makes it less obvious what is happening there, and more complicated to conditionalize or comment out in the future. * The %{?jobs:-j%{jobs}} syntax isn't valid, replace it with %{?_smp_mflags}. * %defattr(-,root,root) should be %defattr(-,root,root,-) * For EL-5 or older, packages containing pkgconfig(.pc) files must Requires: pkgconfig (for directory ownership and usability). * I'm not sure why you have split the glib library into a subpackage. Is this to minimize dependencies on the main binary? * The LICENSE file must be included as %doc as part of every possible install transaction. This means that if a package with LICENSE is pulled in as a dependency of another subpackage, it does not need to be present, but in the case of items like the glib, perl, python subpackages, if they can be installed without the -libs subpackage, then they need to have LICENSE as %doc. * You use both $RPM_BUILD_ROOT and %{buildroot}. Pick one and use it consistently throughout. I recommend %{buildroot}. * Is it really necessary to use %{__make} and %{__cp} instead of simply "make" or "cp"? * You still have FIXME - PHP REQUIRES * Does make check not work for older releases than RHEL 7? * Is there a reason this package isn't planned for Fedora? -- 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