Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.

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 04:23:10 UTC 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-21 02:14:01 UTC
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-21 02:14:46 UTC
See also bug 456032 and bug 456034.

Comment 2 Matthias Clasen 2008-07-21 04:02:21 UTC
Seems you are missing a BR against device-mapper-devel

Comment 3 Matthias Clasen 2008-07-21 04:06:03 UTC
and intltool

Comment 4 Matthias Clasen 2008-07-21 15:00:36 UTC
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 15:07:10 UTC
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 15:22:48 UTC
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 17:23:26 UTC
Thanks for the review; uploaded new SPEC and SRPM at the same location. Does it
look OK?

Comment 8 Matthias Clasen 2008-07-21 18:28:56 UTC
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 18:40:21 UTC
(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 19:50:24 UTC
Looking good now.

Comment 11 David Zeuthen 2008-07-21 20:29:14 UTC
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 15:55:53 UTC
cvs done.