[Bug 234326] Review Request: bandsaw - A syslog monitoring program for the GNOME desktop

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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: bandsaw - A syslog monitoring program for the GNOME desktop


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


dtimms@xxxxxxxxxxxx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dtimms@xxxxxxxxxxxx




------- Additional Comments From dtimms@xxxxxxxxxxxx  2007-07-28 10:56 EST -------
bandsaw review:

(In reply to comment #4)
> new spec: http://glive.tuxfamily.org/fedora/bandsaw/bandsaw.spec
Disclaimer: I am not a reviewer nor sponsor, and this is my first "review like"
submission.

>BuildRequires:  pygtk2-devel, gnome-python2-devel, gnome-doc-utils, gettext,
desktop-file-utils, scrollkeeper
My personal preference is to limit each line to 80 chars. You can have multiple
BR entries, perhaps splitting of the last two/3 items.

> %post
> update-desktop-database &> /dev/null ||:
- I don't know if it makes a difference, but the snippet shows || :

yum localinstall the .src.rpm emits the following:
warning: user damien does not exist - using root
warning: group damien does not exist - using root
  Installing: bandsaw                      ###################### 
[29/30]warning: user damien does not exist - using root
warning: group damien does not exist - using root
- I think this is not a problem (on the fedora buils sys), but can be solved by
installing mock and building the src.rpm as the mock user ?

New lines:
- please be consistent with the new line approach {double} you took between BR
and %description, but haven't continued with all the way through.

spelling: postum

MUST Items:
.x rpmlint result:
W: bandsaw non-conffile-in-etc /etc/gconf/schemas/bandsaw.schemas
E: bandsaw no-binary
  - Please use the output of rpmlint -i for more info to solve these.

./ named according to the Package Naming Guidelines: matches upstream project
and source download name.
./ spec file name matches the base package bandsaw.spec
. package must meet the Packaging Guidelines.
./ package must be licensed with an open-source compatible license:
  - web site indicates GPL and upstream source includes GPLv2.
./ License field in the package spec file must match the actual license:
  - GPL
./ source package includes the text of the license(s) in its own file, so text
of the license(s) for the package must be included in %doc:
  - COPYING is included as required.
./ The spec file must be written in American English.
.? spec file for the package MUST be legible:
  - at this stage, it is not obvious to me the need for:
%define debug_package %{nil}
%{!?python_sitearch: %define python_sitearch %(%{__python} -c "from
distutils.sysconfig import get_python_lib; print get_python_lib(1)")}
  Can you point to an existing fedora spec that uses similar ?
./ source in .src.rpm matches upstream md5sum:
  - md5sum bandsaw-0.3.0.tar.gz /usr/src/redhat/SOURCES/bandsaw-0.3.0.tar.gz 
22312a8bccc283d29db55074c69b6073  bandsaw-0.3.0.tar.gz
22312a8bccc283d29db55074c69b6073  /usr/src/redhat/SOURCES/bandsaw-0.3.0.tar.gz
./ successfully compiles and builds into binary rpms: i386 {athlon}
.? If the package does not successfully compile, build or work on an
architecture - :
  - only tried on i386{i686/athlon} and no excludearchs listed.
  - have you tested on x86_64 or other arch ?
.? build dependencies must be listed in BuildRequires:
  - no listed BR is in the auto included list, the package built on my system
after yum localinstall the .src.rpm installed lots of -devel rpms.
  - yet to try mock build.
.? spec file MUST handle locales properly:
  - neither find_lang macro nor %{_datadir}/locale are used.
./ has no shared library files
./ not relocatable and does not use Prefix: /usr
.? package must own all directories that it creates:
  - doesn't install anything currently.
./ A package must not contain any duplicate files in the %files listing:
  - does not appear to.
.? Permissions on files must be set properly. Executables should be set with
executable permissions, for example. Every %files section must include a
%defattr(...) line.
.? must have a %clean section, containing rm -rf %{buildroot} (or $RPM_BUILD_ROOT):
  - Included. Is there a preference for the %{x} style ?
.? Each package must consistently use macros:
  - debug_package doesn't seem to be used. What is it's purpose ?
./ The package must contain code, or permissable content.
  - contains a GUI app
./ Large documentation files:
  - total doc is 35kB
.x %doc files must not affect the runtime of the application:
  - currently no files are installed at all.
./ Header files must be in a -devel package:
  - no header files.
./ Static libraries must be in a -static package:
  - no static libraries.
./ has no pkgconfig(.pc) files.
./ library files with a suffix: no libraries
./ devel packages must require the base package: 
  - no -devel package
./ Packages must NOT contain any .la libtool archives
  - no .la's
.x Packages containing GUI applications must include a %{name}.desktop file, and
that file must be properly installed with desktop-file-install
>+ desktop-file-install --vendor= --delete-original --dir
/var/tmp/bandsaw-0.3.0-2.fc7-root-root/usr/share/applications
/var/tmp/bandsaw-0.3.0-2.fc7-root-root//usr/share/applications/bandsaw.desktop
/var/tmp/bandsaw-0.3.0-2.fc7-root-root/usr/share/applications/bandsaw.desktop:
warning: The 'Application' category is not defined by the desktop entry
specification.  Please use one of "AudioVideo", "Audio", "Video", "Development",
"Education", "Game", "Graphics", "Network", "Office", "Settings", "System",
"Utility" instead
  - Application category is not defined in:
http://standards.freedesktop.org/menu-spec/latest/apa.html. Please remove.
  - GTK;Monitor; would be additional suitable categories
  - no GenericName= is defined
  - .desktop does not get installed.
  
.? Packages must not own files or directories already owned by other packages. 
.? At the beginning of %install, each package MUST run rm -rf %{buildroot} (or
$RPM_BUILD_ROOT):
  - it includes $RPM_BUILD_ROOT as required, but then uses eg %{_datadir} in the
same command. I think it would make sense to keep to the % method.
./ All filenames in rpm packages must be valid UTF-8.

SHOULD Items:
./ If the source package does not include license text(s) as a separate file
from upstream, the packager SHOULD query upstream to include it: included.
./ The description and summary sections in the package spec file should contain
translations for supported Non-English languages:
  - no other translations available 

.todo package builds in mock.
  - package does not install anything currently; I'll get to this once other
issues have been taken care of.

.?  The package should compile and build into binary rpms on all supported
architectures:
  - this is a python/gtk/glade program. Is python bytecode crossplatform, ie
will it just work on any platform ? If so should it be noarch ?
  
.x package functions as described
  - the package did not install it's bits. typing bands{tab} there is no
autocomplete, and updatedb shows only my /usr/src/ bandsaw files.

.  If scriptlets are used, those scriptlets must be sane. This is vague, and
left up to the reviewers judgement to determine sanity.
.  Usually, subpackages other than devel should require the base package using a
fully versioned dependency.
./ no pkgconfig(.pc) files: python.
./ no file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin.

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

_______________________________________________
Fedora-package-review mailing list
Fedora-package-review@xxxxxxxxxx
http://www.redhat.com/mailman/listinfo/fedora-package-review

[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]