[Bug 193224] Review Request: streamer

[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: streamer


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


fedora at adslpipe.co.uk changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fedora at adslpipe.co.uk




------- Additional Comments From fedora at adslpipe.co.uk  2006-07-11 19:54 EST -------
Review of http://people.redhat.com/pnemade/Qcwebcam/streamer-1.1.3-1.fc5.src.rpm

* rpmlint must be run on every package.

  $rpmlint -v /home/andy/Desktop/streamer-1.1.3-1.fc5.src.rpm
  I: streamer checking

  $ rpmlint -v /usr/src/redhat/RPMS/i386/streamer-1.1.3-1.i386.rpm
  I: streamer checking
  W: streamer incoherent-version-in-changelog 1.1-3-1 1.1.3-1

  $ rpmlint -v /usr/src/redhat/RPMS/i386/streamer-debuginfo-1.1.3-1.i386.rpm
  I: streamer-debuginfo checking

  suggest correcting version 1.1-3-1 to 1.1.3-1 likewise 1.1-2-1 to 1.1.2-1
  otherwise ok

* package must be named according to the guidelines.

  ok

* spec file name must match the base package name

  ok

* package must meet the Packaging Guidelines.

  It amuses me that meeting the packaging guidlines is an item within the
packaging guidelines checklist

* package must be licensed with an open-source compatible license 

  spec file = GPL
  tarball contains licence file = GPL v2  

* The License field in the package spec file must match the actual license.

  most source files themselves do contain copyright, but not licencing info,
  of those that do, only one or two clearly state GPL, 
  others in contrib-plugin directory seem to make a weaker GPL claim.

* If package includes the text of the license(s) in its own file, then that file
must be included in %doc.

  ok

* The spec file must be written in American English.

  ok

* The spec file for the package MUST be legible.

  ok, to be ultrapicky, add a space between "Name:" and "streamer"?
  suggestion, make Description: slightly more, err, descriptive 
  e.g. use "Command line tool for streaming capture, including audio." from the
README file.
 
* The sources used to build the package must match the upstream source

  cannot test because source
  http://people.redhat.com/pnemade/Qcwebcam/streamer-1.1.3.tar.bz2
  does not exist

* The package must successfully compile and build into binary rpms on at least
one supported architecture.

  compiled and build on i386 (FC6T1/rawhide)

* If the package does not successfully compile, build or work on an
architecture, then those architectures should be listed in the spec in
ExcludeArch. Each architecture listed in ExcludeArch needs to have a bug filed
in bugzilla, describing the reason that the package does not compile/build/work
on that architecture. The bug number should then be placed in a comment, next to
the corresponding ExcludeArch line. New packages will not have bugzilla entries
during the review process, so they should put this description in the comment
until the package is approved, then file the bugzilla entry, and replace the
long explanation with the bug number. (Extras Only) The bug should be marked as
blocking one (or more) of the following bugs to simplify tracking such issues:
[WWW] FE-ExcludeArch-x86, [WWW] FE-ExcludeArch-x64, [WWW] FE-ExcludeArch-ppc

  nothing excluded, I can test on FC5/x86_64 later, does the packager know how
it performs on other architectures?

* All build dependencies must be listed in BuildRequires, except for any that
are listed in the exceptions section of Packaging Guidelines; inclusion of those
as BuildRequires is optional. Apply common sense.

  Some BRs are specified, I have not checked the list is exhaustive

* The spec file MUST handle locales properly

  No locales in package

* If the package contains shared library files located in the dynamic linker's
default paths, that package must call ldconfig 

  ok, .so files in own directory only, not added to linker path

* If the package is designed to be relocatable, the packager must state this
fact in the request for review, along with the rationalization for relocation of
that specific package. Without this, use of Prefix: /usr is considered a blocker.

  no such statement, assume this is correct.

* 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. The exception to this are directories listed explicitly in the
Filesystem Hierarchy Standard ([WWW]
http://www.pathname.com/fhs/pub/fhs-2.3.html), as it is safe to assume that
those directories exist.

  ok (/usr/lib/streamer and /usr/share/doc/streamer-version)

* A package must not contain any duplicate files in the %files listing.

  ok, all files explicitly listed, no wildcards used at all
  would it be considered neater to use %{_libdir}/streamer/*.so instead?

* Permissions on files must be set properly. Executables should be set with
executable permissions, for example. Every %files section must include a
%defattr(...) line.

  ok, permissions on files set within rpm, not modified by %defattr or %attr

* package must have a %clean section, which contains rm -rf %{buildroot} (or
$RPM_BUILD_ROOT).

  ok

* package must consistently use macros, as described in the macros section of
Packaging Guidelines.
 
  does use both $ and % macros, but never mixes usage for the same macro, so
meets guidelines

* The package must contain code, or permissable content. This is described in
detail in the code vs. content section of Packaging Guidelines.

  ok, executable + supporting libraries

* Large documentation files should go in a -doc subpackage. (The definition of
large is left up to the packager's best judgement, but is not restricted to
size. Large can refer to either size or quantity)
 
  separate -doc not justified!

* If a package includes something as %doc, it must not affect the runtime of the
application. To summarize: If it is in %doc, the program must run properly if it
is not present.

  ok

* Header files or static libraries must be in a -devel package.

  none

* Files used by pkgconfig (.pc files) must be in a -devel package.

  none

* If a package contains library files with a suffix (e.g. libfoo.so.1.1), then
library files that end in .so (without suffix) must go in a -devel package.

  ok, no suffixes used

* In the vast majority of cases, devel packages must require the base package
using a fully versioned dependency: Requires: %{name} = %{version}-%{release} 

  ok, no devel package

* Packages must NOT contain any .la libtool archives, these should be removed in
the spec.

  ok

* Packages containing GUI applications must include a %{name}.desktop file, and
that file must be properly installed with desktop-file-install in the %install
section. This is described in detail in the desktop files section of Packaging
Guidelines. If you feel that your packaged GUI application does not need a
.desktop file, you must put a comment in the spec file with your explanation.

  ok, not a GUI app

* must not own files or directories already owned by other packages. The rule of
thumb here is that the first package to be installed should own the files or
directories that other packages may rely upon. This means, for example, that no
package in Fedora should ever share ownership with any of the files or
directories owned by the filesystem or man package. If you feel that you have a
good reason to own a file or directory that another package owns, then please
present that at package review time.

  ok

* If the source package does not include license text(s) as a separate file from
upstream, the packager SHOULD query upstream to include it.

  ok, GPL file included

* The description and summary sections in the package spec file should contain
translations for supported Non-English languages, if available.

  none included

* The reviewer should test that the package builds in mock.

  not tested

* The package should compile and build into binary rpms on all supported
architectures.

  only tested on i386/FC6T1(rawhide)

* The reviewer should test that the package functions as described. A package
should not segfault instead of running, for example.

  I do not have appropriate video capture hardware on this machine

* If scriptlets are used, those scriptlets must be sane. This is vague, and left
up to the reviewers judgement to determine sanity.

  Ok, none used.

* Usually, subpackages other than devel should require the base package using a
fully versioned dependency. 
 
  ok, no sub-packages



-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.