[Bug 561470] Review Request: beakerlib - shell-level integration testing library

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

--- Comment #11 from Petr Muller <pmuller at redhat.com> 2010-02-12 11:27:03 EST ---
(In reply to comment #4) 
> Can the .spec summary be made more specific?  Presently, it says "An operating
> system integration testing harness".  Is that the correct summary for
> beakerlib?

I've put there what Petr Splichal proposed in comment 5

> * MUST: rpmlint must be run on every package. The output should be posted in
> the review.
> 
> FAIL - I've attached a small spec patch to address several issues, but you will
> need to adjust further to address the issues identified below.

rpmlint is now silent on the package

> FAIL - 
>  * I can't tell by looking at the code what the license is.  You may wish to
> include a LICENSE file.
>  * The Makefile lists '# License: GPL v2 or later', but the package is listed
> as GPLv2.  if this is the case, you may wish to change the .spec file License:
> GPLv2+
>  * src/staf-rhts/BEAKERLIB.pm shows "Eclipse Public License (EPL) V1.0" which
> is not compatible with the GPLv2 (see
> http://fedoraproject.org/wiki/Licensing#SoftwareLicenses).

staf-rhts was removed. The correct license is GPLv2 only and all files should
have standardized license infirmation.

> * MUST: If (and only if) the source package includes the text of the license(s)
> in its own file, then that file, containing the text of the license(s) for the
> package must be included in <code>%doc</code>.
> 
> No LICENSE file included, so not an issue.

LICENSE now included and tagged by %doc

> FAIL - Please correct according to
> https://fedoraproject.org/wiki/Packaging/SourceURL

Fixed, see previous comments

> * MUST: The package MUST successfully compile and build into binary rpms on at
> least one primary architecture.
> 
> WARN - while the package builds successfully, it won't build properly once the
> %files are adjusted to suitable system-wide locations.  The build process will
> need to be adjusted.

Paths were fixed, see previous comments.

> * MUST: A package must own all directories that it creates. If it does not
> create a directory that it uses, then it should require a package which does
> create that directory.
> 
> FAIL - please see attached patch

Fixed, see previous comment

> * MUST: A Fedora package must not list a file more than once in the spec file's
> %files listings.
> 
> FAIL - please see attached patch

Fixed

> * MUST: Permissions on files must be set properly. Executables should be set
> with executable permissions, for example. Every <code>%files</code> section
> must include a <code>%defattr(...)</code> line.
> 
> FAIL - please see attached patch

I believe all files have right permissions.

> * MUST: Each package must have a %clean section, which contains <code>rm -rf
> %{buildroot}</code> ([[Packaging/Guidelines#UsingBuildRootOptFlags|or
> $RPM_BUILD_ROOT]]).
> 
> WARN - noted earlier in rpmlint output

Fixed.

> * MUST: At the beginning of <code>%install</code>, each package MUST run
> <code>rm -rf %{buildroot}</code>
> ([[Packaging/Guidelines#UsingBuildRootOptFlags|or $RPM_BUILD_ROOT]]).
> 
> FAIL - please see attached patch

Fixed.

> * SHOULD: If the source package does not include license text(s) as a separate
> file from upstream, the packager SHOULD query upstream to include it.
> 
> Pmuller is upstream in this case, I've recommended including a LICENSE file

Included.

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