Bug 456033

Summary: DeviceKit-disks - Disk Management Service
Product: [Fedora] Fedora Reporter: David Zeuthen <davidz>
Component: Package ReviewAssignee: Matthias Clasen <mclasen>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: low Docs Contact:
Priority: low    
Version: rawhideCC: fedora-package-review, mclasen, notting
Target Milestone: ---Flags: mclasen: fedora‑review+
kevin: fedora‑cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-07-24 00:23:10 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Description David Zeuthen 2008-07-20 22:14:01 EDT
Description : DeviceKit-disks provides a daemon, D-Bus API and command line tools
for managing disks and storage devices.

http://people.freedesktop.org/~david/gdu-dkd-review/DeviceKit-disks.spec
http://people.freedesktop.org/~david/gdu-dkd-review/DeviceKit-disks-002-0.git20080720.fc10.src.rpm
Comment 1 David Zeuthen 2008-07-20 22:14:46 EDT
See also bug 456032 and bug 456034.
Comment 2 Matthias Clasen 2008-07-21 00:02:21 EDT
Seems you are missing a BR against device-mapper-devel
Comment 3 Matthias Clasen 2008-07-21 00:06:03 EDT
and intltool
Comment 4 Matthias Clasen 2008-07-21 11:00:36 EDT
rpmlint says:

[mclasen@golem ~]$ rpmlint DeviceKit-disks-*
DeviceKit-disks.i386: E: executable-sourced-script
/etc/profile.d/devkit-disks-bash-completion.sh 0755
DeviceKit-disks.i386: W: non-conffile-in-etc /etc/udev/rules.d/95-devkit-disks.rules
DeviceKit-disks.i386: W: non-conffile-in-etc
/etc/dbus-1/system.d/org.freedesktop.DeviceKit.Disks.conf
3 packages and 0 specfiles checked; 1 errors, 2 warnings.
Comment 5 Matthias Clasen 2008-07-21 11:07:10 EDT
The to conffile warning can probably be ignored, if it is common practise to
treat udev rules not as conffiles.

The executable-sourced-script warning should probably be fixed


Comment 6 Matthias Clasen 2008-07-21 11:22:48 EDT
package name: ok
spec file name: ok
packaging guidelines:
 - the source tag points to a nonexisting file. Should just make it a filename
   and add a comment explaining that this is a git snapshot (see
   https://fedoraproject.org/wiki/Packaging/SourceURL)
 - the handling of %doc content should be straightened out
license: ok
license field: ok
license file: ok
spec language: ok
spec legibility: ok
upstream sources: ok
buildable: yes
ExcludeArch: n/a
BuildRequires: see above, missing device-mapper-devel and intltool
locale handling: n/a
ldconfig: n/a
relocatable: n/a
directory ownership: 
  - must not own /usr/share/PolicyKit/policy
duplicate files: ok
permissions: ok
%clean: ok
macro use: ok
content: permissible
large docs: n/a
%doc content: ok
header files: n/a
static libraries: n/a
pc files: n/a
shared libraries: n/a
devel deps: ok
libtool archives: ok
gui apps: n/a
directory ownership:
  - see above about /usr/share/PolicyKit/policy
%install: ok
utf8 filenames: ok
Comment 7 David Zeuthen 2008-07-21 13:23:26 EDT
Thanks for the review; uploaded new SPEC and SRPM at the same location. Does it
look OK?
Comment 8 Matthias Clasen 2008-07-21 14:28:56 EDT
Ah, one I overlooked:

%dir %{_datadir}/dbus-1/interfaces

You shouldn't own that directory either. Instead, fix dbus to own it.
Comment 9 David Zeuthen 2008-07-21 14:40:21 EDT
(In reply to comment #8)
> Ah, one I overlooked:
> 
> %dir %{_datadir}/dbus-1/interfaces
> 
> You shouldn't own that directory either. 

Fine. Fixed. Uploaded the new SPEC and SRPM. Thanks.


Comment 10 Matthias Clasen 2008-07-21 15:50:24 EDT
Looking good now.
Comment 11 David Zeuthen 2008-07-21 16:29:14 EDT
New Package CVS Request
=======================
Package Name: DeviceKit-disks
Short Description: Disk Management Service
Owners: davidz
Branches:
InitialCC:
Cvsextras Commits: yes
Comment 12 Kevin Fenzi 2008-07-22 11:55:53 EDT
cvs done.