[Bug 734275] Review Request: aqemu - A QT graphical interface to QEMU and KVM

[bugzilla at redhat.com]

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 at redhat.com> 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.