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=734275 --- Comment #21 from Stephen Gordon <sgordon@xxxxxxxxxx> 2012-02-28 13:14:43 EST --- (In reply to comment #17) Updated files, responses inline: Spec URL: http://sgordon.fedorapeople.org/repo/SOURCES/aqemu-0.8.2-6.spec SRPM URL: http://sgordon.fedorapeople.org/repo/SRPMS/aqemu-0.8.2-6.fc16.src.rpm > Here's the formal review of your package. There are a couple of things that > need to be fixed: > > - The package currently doesn't build in mock (F16) because of > * missing BR: gnutls-devel Added BuildRequires. > * clash of "error" macros > I suggest to manually expand the macro in file > Embedded_Display/vncview.cpp and drop its definition from the same file. > This should be reported upstream. Updated upstream change applied as patch for now (see comment # 20), will drop the patch after next stable release. > - The license of aqemu is GPLv2+ according to the source file headers. > => update the License field accordingly Updated. > - Add a short comment above Patch0 telling what the patch does. Have you sent > the patch upstream? Updated, also included comment for the additional patch. > - File README contains installation instructions only. > => it should be removed from the package Removed. > - Please add the section suffix to the manpage filename in %files: > %{_mandir}/man1/%{name}* => %{_mandir}/man1/%{name}.1* Updated to include the section suffix. > - Add blank lines between the %changelog entries to improve legibility. Updated. Going through the needs work items from the checklist just to confirm: > --------------------------------- > key: > > [+] OK > [.] OK, not applicable > [X] needs work > --------------------------------- > > [X] MUST: The License field in the package spec file must match the actual > license. > [X] MUST: All build dependencies must be listed in BuildRequires. Actioned as above. > EPEL <= 5 only: > [X] MUST: The spec file must contain a valid BuildRoot field. > [X] MUST: At the beginning of %install, each package MUST run rm -rf > %{buildroot}. > [X] MUST: Each package must have a %clean section, which contains rm -rf > %{buildroot}. > [.] MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' I do not intend to package for EPEL at this time. > separate file from upstream, the packager SHOULD query upstream to include it. > [X] SHOULD: All patches should be commented in the spec file > [X] SHOULD: The reviewer should test that the package builds in mock. Actioned as above. -- 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