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



--- Comment #2 from Steve Grubb <sgr...@redhat.com> ---
Thanks for the review. New SRPM and spec file reposted.

(In reply to Robert-André Mauchin from comment #1)
> Hello,
> 
>  - Not used in Fedora anymore:
> 
> Group:
> 
> BuildRoot:
> 
> rm -rf $RPM_BUILD_ROOT
> 
> %defattr(-,root,root,-)

Removed


>  - Use the correct systemd macros:
> 
> %{?systemd_requires}
> BuildRequires: systemd

Removed -devel


>  - the license file must be included with %license, not %doc:
> 
> %files
> %doc README
> %license COPYING

Fixed

>  - These %attr(644,root,root) %attr(755,root,root) are not needed because
> they are already the default value.

I like explicit permissions in case the make file has an accident. There are a
number of executable man pages on Fedora. :-)


>  - COPYING contains GNU General Public License v3.0 but your license header
> contains GPLv2+. Please use GPLv3 or GPLv3+.

Doh! Fixed.


>  - Please follow
> https://fedoraproject.org/wiki/Packaging:UsersAndGroups#Dynamic_allocation
> to add user and group.
>    Add Requires(pre): shadow-utils

Fixed.

> %pre
> getent group GROUPNAME >/dev/null || groupadd -r GROUPNAME
> getent passwd USERNAME >/dev/null || \
>     useradd -r -g GROUPNAME -d HOMEDIR -s /sbin/nologin \
>     -c "Useful comment about the purpose of this account" USERNAME
> exit 0

I think the existing code should do the trick. The default behavior is to
create both user and group unless overridden. So, checking the user should
suffice for simple cases.

>  - In %files, for man pages, don't directly link to the gz extension but use
> a glob:
> 
> %{_mandir}/man8/fapolicyd.8.*
> %{_mandir}/man5/fapolicyd.rules.5.*
> 
> See https://fedoraproject.org/wiki/Packaging:Guidelines#Manpages for
> rationale.

Fixed. I widened the glob as there will certainly be future man pages added.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

Reply via email to