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: --- Target Upstream Version:
Embargoed:

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.