[Bug 1131127] Review Request: safelease - legacy locking mechanism for VDSM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1131127



--- Comment #7 from Michael Schwendt <bugs.michael@xxxxxxx> ---
A few comments:

> %global libname safelease
> 
> Name:       %{libname}

What's the purpose of defining %libname when

1) the "Name" tag defines %name already,
2) you don't use %libname everywhere,
3) you use %name, %libname *and* a hardcoded "saferelease"

?

Shorter, more common, and cleaner:

  Name: saferelease

That defines %{name} to be "saferelease".
Then use %{name} everywhere instead of "saferelease".
This also works well when renaming the package.


> License:    GPLv2+

That doesn't match the C source file, which contains a GPLv3+ preamble. Please
clarify.


> Source:     %{libname}-%{version}.tar.gz

https://fedoraproject.org/wiki/Packaging:Guidelines#Tags
https://fedoraproject.org/wiki/Packaging:SourceURL#Referencing_Source


> Summary:    Legacy locking utility

> %description
> Legacy locking utility for VDSM

https://fedoraproject.org/wiki/Packaging:Guidelines#Summary_and_description

Could you think of expanding on this a bit more and making it a full sentence?
Currently %description is %summary plus two words only. There is no
documentation included either.


> %files
> %doc AUTHORS COPYING README
> %{_libexecdir}/safelease/safelease

$ rpmls -p safelease-1.0-3.fc21.x86_64.rpm 
-rwxr-xr-x  /usr/libexec/safelease/safelease
drwxr-xr-x  /usr/share/doc/safelease
-rw-r--r--  /usr/share/doc/safelease/AUTHORS
-rw-r--r--  /usr/share/doc/safelease/COPYING
-rw-r--r--  /usr/share/doc/safelease/README

Directory /usr/libexec/saferelease is not included yet. Notice the missing 'd'
entry in the list of files.

https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership
https://fedoraproject.org/wiki/Packaging:UnownedDirectories

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]