[Bug 165985] Review Request: Aeryn - A C++ testing framework

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

Summary: Review Request: Aeryn - A C++ testing framework


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=165985





------- Additional Comments From bugs.michael at gmx.net  2005-08-20 19:55 EST -------
Okay, let's start somewhere. A first rather brief look:

Package builds as "aeryn2-2-1.1.i386.rpm" although the upstream version
is "2.1.1", so why not name the package "aeryn" with version "2.1.1"
and release "1"?

> $ rpmls aeryn2 | grep ^d
> drwxr-xr-x  /usr/include/aeryn/aeryn
> drwxr-xr-x  /usr/include/aeryn/aeryn/details

This reveals that the directory

  /usr/include/aeryn/

is not included in the package. A look at the complete package listing
reveals that

  /usr/lib/aeryn/

is not included either.

Further, /usr/bin/TestClient is a rather generic name. It doesn't look
like a useful binary that should be included, as it only runs a test suite
which most likely ought to be ran during rpmbuild in %check or so instead.
It looks like it returns a proper exit code for "failed/successful" state.

> $ rpm -qd aeryn2
> $

No documentation included at all?

[...]

Now to the spec file (I usually start a review at the spec level, but moved
it here):

> Summary: Aeryn2 is a C++ unit test package

Better IMO  =>  Summary: C++ unit testing framework

> Name: aeryn2
> Version: 2
> Release: 1.1%{?dist}

As mentioned above, I believe this should be "Name: aeryn", "Version: 2.1.1"
and "Release: 1%{?dist}".

> Source0: http://www.all-the-johnsons.co.uk/aeryn/aeryn2.tar.bz2

This was a surprise, since the Source0 URL should point to the upstream
projects download location. It seems you repackaged upstream author's ZIP
archive and included the PDF documentation file in the tarball. Why not
just take his ZIP as "Source0" and put the PDF into "Source1"? Sort of:

%prep
%setup -T -c -n %{name}
unzip %{SOURCE0}

and everything you did to build your tar.bz2 should be done after
extracting the zip.

> $ rpmlint aeryn2-2-1.1.src.rpm 
> E: aeryn2 description-line-too-long

Keep %description lines below 80 characters.

> %build
> make %{?_smp_mflags}

Code compiles without $RPM_OPT_FLAGS, e.g.

> g++ -W -Wall -Werror -pedantic -Wcast-qual -Wcast-align -Wwrite-strings
> -Winline -finline-limit=1048576 -g3 -c -o noncopyable.o
> ../src/noncopyable.cpp -I../include -DNO_OUTPUT_OPERATOR_DETECTION

It may be necessary to patch ./make/common.mk and adjust CXXFLAGS there.

> %clean
> [ "x${RPM_BUILD_ROOT}" != "x/" ] && rm -rf "${RPM_BUILD_ROOT}"

Just "rm -rf $RPM_BUILD_ROOT" is enough and more readable. You do define
a default buildroot. Rare users, who rpmbuild as superuser and redefine
the buildroot play with fire anyway.

> %files
> %defattr(-, root, root)
> %{_bindir}/*
> %{_includedir}/aeryn/*
> %{_libdir}/aeryn/*

Instead, these two

%{_includedir}/aeryn/
%{_libdir}/aeryn/

would also include the two directories and not just their contents.


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.