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=772608 Jorge A Gallegos <kad@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kad@xxxxxxxxx --- Comment #11 from Jorge A Gallegos <kad@xxxxxxxxx> 2012-02-01 03:19:25 EST --- ** I AM NOT A SPONSOR ** However, I'll try and submit a review as per Steven's request. First, I can see a glaring issue with your package, you are shipping GDM's .src.rpm as part of your own source. That is probably not going to fly, even though I can't really find anything in the packaging guidelines that prevents you from doing it (closest would be http://fedoraproject.org/wiki/Packaging/Guidelines#No_inclusion_of_pre-built_binaries_or_libraries but there's no mention about .src.rpm). I see some problems with your approach: - You are shipping a specific version of source that is already present in fedora - You will have to ship newer, static .src.rpm whenever GDM gets a newer version - You may end up fighting duplicated libraries The approach, in my opinion, would be to get in touch with the GDM packagers and ask for your stuff to be included if it's not already. It is not entirely apparent why requiring gdm-devel wouldn't work right now. Now, on with the revision proper: [kad@propane rpmbuild ]$ rpmlint SPECS/ovirt-guest-agent.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. [kad@propane rpmbuild ]$ rpmlint SRPMS/ovirt-guest-agent-1.0.0-2.fc16.src.rpm ovirt-guest-agent.src: W: summary-not-capitalized C oVirt Guest Agent 1 packages and 0 specfiles checked; 0 errors, 1 warnings. ("oVirt" is not capitalized in the main oVirt package, so I guess that warning is ok) [BLOCKER] The License field in the package spec file must match the actual license - the COPYING file in the source tarball has GPLv3, you have GPLv2+ in the spec file [BLOCKER] The sources used to build the package must match the upstream source, as provided in the spec URL - You should take a look at http://fedoraproject.org/wiki/Packaging/SourceURL#Using_Revision_Control and not use a self-hosted source tarball (Source0: http://ghammer.fedorapeople.org/%{name}-%{version}.tar.bz2) [BLOCKER] The package MUST successfully compile and build into binary rpms on at least one primary architecture - I got this error when trying a mock build: xorg-x11-server-Xorg is needed by gdm-1:3.2.1.1-6.fc16.x86_64. [BLOCKER] Each package must consistently use macros - I see a mix of $MACRO and %{macro}, use one style and stick with. Also, if you don't plan on supporting EPEL/RH, you should do away with the whole RPM_BUILD_ROOT [citation needed, maybe?] Additionally, it seems you include some python code in there, so, using http://fedoraproject.org/wiki/Packaging:Python as guide: [BLOCKER] To build a package containing python2 files, you need to have BuildRequires: python2-devel Some additional suggestions: - defattr is not needed anymore, if I recall - you should name your different targets appropriately like "ovirt-guest-agent-gdm-plugin" instead of just "gdm-plugin" - in your %pre step, you add a user rhevagent if it doesn't exist, but you are also hardcoding the UID. You should leave that to the system to figure out and just use -r for system users -- 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