[Bug 718479] Review Request: wmwave - Statistics about a current wireless Ethernet connection

[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=718479

Martin Gieseking <martin.gieseking at uos.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |martin.gieseking at uos.de
         AssignedTo|nobody at fedoraproject.org    |martin.gieseking at uos.de
               Flag|                            |fedora-review?

--- Comment #4 from Martin Gieseking <martin.gieseking at uos.de> 2011-08-02 16:26:51 EDT ---
I'm going to sponsor Damian, thus taking over the review. 


(In reply to comment #3)
> [+] MUST: The package must be licensed with a Fedora approved license.
>     - GPLv2 according to script header

As neither the source file headers nor the documentation mention the intended
GPL version, the package is licensed under GPL+. 

Also see the comment on "GPL+" at
http://fedoraproject.org/wiki/Licensing#SoftwareLicenses


> [] MUST: The spec file must be written in American English.
>     - Spec file includes German Summary(de): and %description -l de

That's fine. Additional translations of Summary and %description are always
welcome.


> [] MUST: Permissions on files must be set properly.
>     is chmod -x %{name}.1 strictly necessary?

Yes. Mario, please clear the exec bits of the manpage:
- drop chmod -x %{name}.1 from %install
- add option -m 644 to the last install statement

I also suggest to move the "find" statement from %build to %prep.


> [+] MUST: Each package must consistently use macros.

It's recommended to either use $RPM_OPT_FLAGS/$RPM_BUILD_ROOT or
%{optflags}/%{buildroot} and not to mix styles (variables vs. macros). 
Currently, %{optflags} are used in the %build section and $RPM_BUILD_ROOT in
%install.


> EPEL <= 5 only:
> [+] MUST: The spec file must contain a valid BuildRoot field.

There's no BuildRoot field in the spec. ;) But that's OK as Mario probably
don't want to build for EPEL.

> [] SHOULD: Timestamps of files should be preserved.

It's good practice to keep the timestamps of files that go from the source
archive into the package, e.g. manpages, media files, ...
The manpage is properly installed with "install -p", so this is OK.


> [] SHOULD: The reviewer should test that the package functions as described.

Just try to install and to run the built binary and verify if it works. It
shouldn't crash at least. It seems to work as expected.

So far for now. I'm going to do the formal review tomorrow.

-- 
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.