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=756046 --- Comment #4 from David Zeuthen <davidz@xxxxxxxxxx> 2011-11-28 10:56:43 EST --- Hey, thanks for the review. I've uploaded new files http://people.freedesktop.org/~david/udisks2.spec http://people.freedesktop.org/~david/udisks2-1.90.0-0.git20111128.fc16.src.rpm Here's a unified diff of the spec file changes made: http://people.freedesktop.org/~david/udisks2.spec.diff Inline comments: (In reply to comment #1) > You should probably refer to 'udisks2' in the %description of the main package, > as well as the subpackage descriptions. Or maybe just add 'This package > contains version 2 of udisks.' for the main package. I've added "This package is for the udisks 2.x series" to all package descriptions. > %defattr(-,root,root,-) is no longer needed Nuked. > There have been requests before to move the bash completion to > /etc/bash-completion.d (https://bugzilla.redhat.com/show_bug.cgi?id=584569). > Something to consider, maybe. OK, don't really agree but not worth fighting. Changed upstream. (In reply to comment #2) > Building in mock reveals a missing > > BuildRequires: gobject-introspection-devel > > After adding that line, it builds ok. Fixed. > rpmlint output: > > rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/udisks2-*.rpm > udisks2.src: W: spelling-error %description -l en_US udisks -> disks, u disks, > Saudis > udisks2.src:98: E: hardcoded-library-path in /lib/udisks2/udisksd > udisks2.src: W: file-size-mismatch udisks-1.90.0.git20111122.tar.bz2 = 584775, > http://people.freedesktop.org/~david/udisks-1.90.0.git20111122.tar.bz2 = 640107 > udisks2.x86_64: W: unexpanded-macro dependency dbus >= %{dbus_version} > %{dbus_version} > udisks2.x86_64: W: spelling-error %description -l en_US udisks -> disks, u > disks, Saudis > udisks2.x86_64: E: executable-sourced-script > /etc/profile.d/udisksctl-bash-completion.sh 0755L > udisks2.x86_64: E: non-standard-dir-perm /var/lib/udisks2 0700L > udisks2.x86_64: W: non-conffile-in-etc > /etc/dbus-1/system.d/org.freedesktop.UDisks2.conf > udisks2.x86_64: W: no-manual-page-for-binary umount.udisks2 > 3 packages and 0 specfiles checked; 3 errors, 6 warnings. > > Some things to fix here: > > - The hardcoded library path is fine, but you need to add a > %dir /lib/udisks2 to own the directory. Done. > - The file size mismatch should be investigated and fixed. I did a new tarball with the same name but forgot to put it online. > - You need to define dbus_version Done. > - Should make /etc/profile.d/udisksctl-bash-completion.sh non-executable > (and, as already mentioned, consider moving it) Done upstream, see above. > - You should probably add a comment in the file list explaining > the permissions of /var/lib/udisks2 Added. (In reply to comment #3) > Review checklist: > > package name: ok > spec file name: ok > packaging guidelines: ok, apart from whats mentioned in the previous comment > license: ok > license field: ok > license file: ok > spec language: ok > spec readable: ok > upstream sources: see the rpmlint complaint above Should be fixed. > buildable: ok, module missing BR Fixed. > excludearch: ok > buildrequires: see initial comment, needs gobject-introspection-devel Fixed. > locale handling: ok > ldconfig: ok > system libs: ok > relocatable: ok > dir ownership: see above, must own /lib/udisks2 Fixed. > duplicate files: ok > permissions: ok. note that defattr() is no longer needed Fixed. > macro use: ok > permissible content: ok > large docs: ok > %doc content: ok > headers: ok > static libs: ok > shared libs: ok > devel deps: ok > libtool archives: ok > gui apps: ok > file ownership: ok > utf8 filenames: ok Thanks for the review. -- 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review