[Bug 579230] Review Request: upnp-inspector - UPnP Device and Service analyzer

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


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

Alex Orlandi <nyrk71@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nyrk71@xxxxxxxxx

--- Comment #1 from Alex Orlandi <nyrk71@xxxxxxxxx> 2010-04-03 17:04:31 EDT ---
Hi Andrea, I'm very glad to review your package.

The review is mostly based on MUST/SHOULD checklist published in the
ReviewGuidelines.

Notation:
[+]: OK
[-]: KO
[?]: doubt
[N]: Not applicable

Review:

[+]:rpmlint run (clean) on every package with the following output:
 rpmlint upnp-inspector.spec
../RPMS/noarch/upnp-inspector-0.2.2-1.fc12.noarch.rpm
../SRPMS/upnp-inspector-0.2.2-1.fc12.src.rpm 
 2 packages and 1 specfiles checked; 0 errors, 0 warnings.  
[?]:The package must be named according to the Package Naming Guidelines (see
below)
[+]:The spec matches the base package %{name}
[+]:The package meets the Packaging Guidelines.
[+]:License is MIT (OK) .
[+]:The License field in the package spec matches the actual license.
[+]:LICENCE file contains text of "MIT, Modern Style with sublicense" and is
included in %doc.
[+]:The spec file must be written in American English.
[+]:The spec file for the package MUST be legible.
[+]:The sources used to build the package matches the upstream source, as
provided in the spec URL.
 md5sum ../SOURCES/UPnP-Inspector-0.2.2.tar.gz /tmp/UPnP-Inspector-0.2.2.tar.gz 
 6823a57f4501c8af6a7b8547b8133759  ../SOURCES/UPnP-Inspector-0.2.2.tar.gz
 6823a57f4501c8af6a7b8547b8133759  /tmp/UPnP-Inspector-0.2.2.tar.gz 
[+]:The package successfully compiles and builds into binary rpms on x86;
considering that is a noarch package may be sufficient.
[N]:If the package does not successfully compile, build or work on an
architecture, then those architectures should be listed in the spec in
ExcludeArch. [...] 
[+]:All build dependencies are listed in BuildRequires, except for any that are
listed in the exceptions section of the Packaging Guidelines.
[N]:The spec file MUST handle locales properly.
[N]:Every binary RPM package (or subpackage) which stores shared library files
(not just symlinks) in any of the dynamic linker's default paths, must call
ldconfig in %post and %postun.
[+]:Package doesn't bundle copies of system libraries.
[+]:The package is not relocatable
[+]:The package owns 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.
[+]:A Fedora package must not list a file more than once in the spec file's
%files listings.
[+]:Permissions on files are set properly. 
[+]:Use of macros is consistent
[+]:The package contains code and permissable content.
[+]:-doc package not needed: doc stuff is almost inexistent :-)
[+]:doc stuff doesn't affect the runtime of the application. 
[N]:Header files must be in a -devel package.
[N]:Static libraries must be in a -static package.
[N]: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.
[N]:[...] devel packages must require the base package using a fully versioned
dependency [...]
[N]:Packages must NOT contain any .la libtool archives, these must be removed
in the spec if they are built.
[+]:Package includes upnp-inspector.desktop file properly installed with
desktop-file-install
[+]:Packages does not own files or directories already owned by other packages.
[+]:rm -rf $RPM_BUILD_ROOT at the beginning of %install
[+]:all filenames in rpm packages are valid UTF-8.

SHOULD items

[N]: If the source package does not include license text(s) as a separate file
from upstream, the packager SHOULD query upstream to include it.
[N]:The description and summary sections in the package spec file should
contain translations for supported Non-English languages, if available
[+]:Builds on mock 
[+]:The package build on all supported architectures [is noarch] (see
http://koji.fedoraproject.org/koji/taskinfo?taskID=2093598) 
[+]:The program starts correctly  (unfortunately I don't have a DLNA so I can't
check more in depth)
[+]:If scriptlets are used, those scriptlets must be sane. This is vague, and
left up to the reviewers judgement to determine sanity.
[N]:Usually, subpackages other than devel should require the base package using
a fully versioned dependency.
[+]:No pkgconfig(.pc) files (no -devel package needed) 
[+]:Package doesn't have file dependencies outside of /etc, /bin, /sbin,
/usr/bin, or /usr/sbin 
[-]:package doesn't contain man pages (see below)

Two issues (not blocking):

* Package name should be ok, considered that "[...] case should only be used
where necessary."
Anyway guidelines say "Keep in mind to respect the wishes of the upstream
maintainers. [...] However, if they do not express any preference of case, you
should default to lowercase naming.".
It could be good to consider to check upstream's preference.

*Package doesn't contain man pages and there's no online help in the
application: consider to work with upstream to add man pages (or doc in the
help menu of the program)

-- 
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.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review

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