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=538327





--- Comment #28 from manuel wolfshant <[email protected]>  2009-12-02 
07:49:17 EDT ---
Couple of things, after a quick glance over the spec
- Unless you have very very good reasons to not do it, please consider using
%dist in the release field. It'll save you some headaches later
- I do not see the reason for 
# Create an empty otppasswd file
[ ! -f $RPM_BUILD_ROOT/etc/otppasswd ] && echo "" >
$RPM_BUILD_ROOT/etc/otppasswd
Since you are in a clean buildroot, the file does not exist unless make install
created it. And a simple "touch $RPM_BUILD_ROOT/etc/otppasswd" seems more
elegant to me. Or even "> $RPM_BUILD_ROOT/etc/otppasswd" (no need for the "echo
" part ). However, nothing incorrect in your approach either. Just a matter of
style
- Please consider the comment #26. It's ugly to restart a daemon which was not
specifically allowed to be on by the admin

-- 
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.

_______________________________________________
Fedora-package-review mailing list
[email protected]
http://www.redhat.com/mailman/listinfo/fedora-package-review

Reply via email to