Bug 757863 - (kwallet) Review Request: kwallet - Manage KDE passwords
Review Request: kwallet - Manage KDE passwords
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christoph Wickert
Fedora Extras Quality Assurance
Depends On:
Blocks: kde-reviews
  Show dependency treegraph
Reported: 2011-11-28 15:46 EST by Rex Dieter
Modified: 2011-12-01 09:43 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2011-12-01 09:43:05 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
cwickert: fedora‑review+
limburgher: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Rex Dieter 2011-11-28 15:46:12 EST
Spec URL: http://rdieter.fedorapeople.org/rpms/kdeutils/kwallet.spec
SRPM URL: http://rdieter.fedorapeople.org/rpms/kdeutils/kwallet-4.7.80-1.fc16.src.rpm
KDE Wallet Manager is a tool to manage the passwords on your KDE system.

This is being split out in kde-4.7.80 from previously monolithic kdeutils
Comment 1 Christoph Wickert 2011-11-28 16:05:15 EST
I'll review this.
Comment 2 Christoph Wickert 2011-11-28 16:56:03 EST
REVIEW FOR e17a4eb2bbf64049d1c7570b65fafdcd  kwallet-4.7.80-1.fc16.src.rpm

MUST items

OK - MUST: rpmlint output: rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/kwallet-*
kwallet.src: W: invalid-url Source0: ftp://ftp.kde.org/pub/kde/stable/4.7.80/src/kwallet-4.7.80.tar.bz2 <urlopen error ftp error: 550 Failed to change directory.>
kwallet.x86_64: E: script-without-shebang /usr/share/applications/kde4/kwalletmanager.desktop
kwallet.x86_64: E: script-without-shebang /usr/share/applications/kde4/kwalletmanager-kwalletd.desktop
kwallet.x86_64: W: no-manual-page-for-binary kwalletmanager
3 packages and 0 specfiles checked; 2 errors, 2 warnings.

Source URL needs to be fixed, the rest is ok. The desktop files are executable on purpose.

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 3ec35007551a6f0fdc2be3d3c431e0d5
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: kcm_kwallet.so does not use versioning, but is an exception
N/A - MUST: -devel package requires the base package using a fully versioned dependency
OK - MUST: package does not contain any .la libtool archives
FIX - MUST: package contains a GUI application and a %{name}.desktop file but this file is not properly installed or validated desktop-file-utils
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'.

- BR desktop-file-utils
- kwalletmanager.desktop contains a MimeType but the package lacks the scriptlet from http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#desktop-database
- use desktop-file-install or desktop-file-validate in %install


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

Please fix the issues and consider the package

Comment 3 Christoph Wickert 2011-11-28 17:22:43 EST
(In reply to comment #2)
> - BR desktop-file-utils

already in place, please ignore
Comment 4 Rex Dieter 2011-11-29 09:16:40 EST
New Package SCM Request
Package Name: kwallet
Short Description: Manage KDE passwords
Owners: than jreznik ltinkl rnovacek rdieter kkofler
Branches: f16
Comment 5 Gwyn Ciesla 2011-11-29 09:24:23 EST
Git done (by process-git-requests).
Comment 6 Rex Dieter 2011-12-01 09:43:05 EST
thanks!  imported into git (not built yet until all kdeutils modules are ready).

Note You need to log in before you can comment on or make changes to this bug.