[Bug 757860] Review Request: kgpg - Gpg Gui tool

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

Christoph Wickert <cwickert at fedoraproject.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+

--- Comment #2 from Christoph Wickert <cwickert at fedoraproject.org> 2011-11-28 18:14:59 EST ---
REVIEW FOR 55ed39ab549b6cc50d4929d7a4498cc4  kgpg-4.7.80-1.fc16.src.rpm


MUST items

FIX - MUST: rpmlint output: $ rpmlint
/var/lib/mock/fedora-rawhide-x86_64/result/kgpg-*
kgpg.src:8: W: macro-in-comment %{name}
kgpg.src: W: invalid-url Source0:
ftp://ftp.kde.org/pub/kde/stable/4.7.80/src/kgpg-4.7.80.tar.bz2 <urlopen error
ftp error: 550 Failed to change directory.>
kgpg.x86_64: E: script-without-shebang
/usr/share/applications/kde4/kgpg.desktop
kgpg.x86_64: W: no-manual-page-for-binary kgpg
3 packages and 0 specfiles checked; 1 errors, 3 warnings.

Source URL needs to be fixed, the rest is ok. The desktop files are executable
on purpose.
The macro-in-comment warning is caused by the  commented out URL line; remove
if it not going to be used anymore.

OK - MUST: package is named according to the Package Naming Guidelines
OK - MUST: spec file name matches the base package in the format %{name}.spec
OK - MUST: package meets the Packaging Guidelines
OK - MUST: package is licensed with a Fedora approved license and meets the
Licensing Guidelines: GPLv2+
OK - MUST: License field in the package spec file matches the actual license:
GPLv2+
OK - MUST: source package includes the text of the license in its own file and
that file is included in %doc
OK - MUST: spec file is written in American English
OK - MUST: spec file for the package is legible
OK - MUST: sources match the upstream source, as provided in the spec URL by
md5 b3731775d46443d429fccd6e0c9eb0d9
OK - MUST: package successfully compiles and builds into binary rpms on at
least one primary architecture
N/A - MUST: If the package does not successfully compile, build or work on an
architecture, then those architectures should be listed in the spec in
ExcludeArch
OK - MUST: all build dependencies are listed in BuildRequires
OK - MUST: spec file handles locales properly using %find_lang
N/A - MUST: package (or subpackage) stores shared library files in the dynamic
linker's default paths and call ldconfig in %post and %postun
OK - MUST: package does not bundle copies of system libraries
OK - MUST: package is not designed to be relocatable
OK - MUST: package owns all directories that it creates
OK - MUST: package does not list a file more than once in the spec file's
%files listings
OK - MUST: permissions on files are set properly
OK - MUST: package consistently use macros
OK - MUST: package contains code, or permissable content
N/A - MUST: large documentation files must go in a -doc subpackage
OK - MUST: files included as %doc do not affect the runtime of the application
N/A - MUST: header files are in -devel package
N/A - MUST: static libraries are in -static package
N/A - MUST: library files with a suffix are in -devel package
N/A - MUST: -devel package requires the base package using a fully versioned
dependency
OK - MUST: package does not contain any .la libtool archives
OK - MUST: package contains a GUI application and includes a %{name}.desktop
file that is properly validated with desktop-file-validate
OK - MUST: package does not own files or directories already owned by other
packages
OK - MUST: all filenames in the package are valid UTF-8


SHOULD items

N/A - SHOULD: source package does not include license text(s) as a separate
file from upstream, query upstream to include it
N/A - SHOULD: description and summary sections should contain translations for
supported Non-English languages, if available
OK - SHOULD: package builds in mock
OK - SHOULD: package compiles and builds into binary rpms on all supported
architectures
OK - SHOULD: package functions as described
OK - SHOULD: scriptlets are sane
N/A - SHOULD: subpackages other than devel require the base package using a
fully versioned dependency
N/A - SHOULD: pkgconfig(.pc) files are in -devel package
OK - SHOULD: package has no file dependencies outside of /etc, /bin, /sbin,
/usr/bin, or /usr/sbin
N/A - SHOULD: package contains man pages for binaries/scripts


OTHER items

OK - latest (un)stable version packaged
FIX - source URL is invalid: 'stable' should be 'unstable'
OK - compiler flags ok
OK - debuginfo complete
N/A - package contains a pkgconfig(.pc) files and has 'Requires: pkgconfig'.


ISSUES

- Fix Soure0 URL
- Please add a comment to the Patch, the URL of the bug for upstreaming it or
the URL of the commit where it was applied.
- desktop-file-validate should be in %install not in %check
- package requires dbus-launch, but I am not sure there is an indirect
dependency on dbus-x11 already. Consider adding a direct one.


NOTES

- %setup -q -n %{name}-%{version} is the same as %setup -q
- consider adding a %clean section and a %defattr line for compatibility with
older versions of rpm
- The license is a mix of GPLv2+ and GPLv3+. This can be summarized under
GPLv2+, but IANAL and don't know what the KDE SIG usually does about the "any
later version accepted by the membership of KDE e.V" phrase.


Please fix the issues and consider the package

APPROVED

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