https://bugzilla.redhat.com/show_bug.cgi?id=1310092 --- Comment #8 from Richard Shaw <hobbes1069@xxxxxxxxx> --- Ok, another round of spec comments. You may have to do some explaining as this is not an area I'm familiar with but you're doing a lot of interesting things with this package. 1. Have you considered a systemd timer rather than a cron job? 2. %global cryptobonedir %{_prefix}/lib/%{name} It's odd to place this between %description and %prep. I usually throw all this stuff at the top of the spec file so it sticks out. 3. It would be good to add a comment on why this is necessary: if ! systemctl is-active postfix > /dev/null ; then systemctl enable postfix fi 4. This should be done in %install cp %{cryptobonedir}/GUI/cryptobone.jpg /usr/share/icons/default desktop-file-install --dir /usr/share/applications -m 755 %{cryptobonedir}/GUI/cryptobone.desktop 5. chkconfig --add cryptoboned SysV init scripts are just redirected to systemd, I would think this would only be useful on EL 6. 6. echo "Please reboot your computer as the crypto bone daemon will start only while booting." Generally output during install is discouraged and anyone using a GUI installer won't see it. 7. What is the purpose of this? Placing systemd unit files in /etc is intended for the end user so they can override the files in /usr/lib/systemd/system. I'm pretty sure mucking around with /etc/systemd/system is forbidden. if [ -f /etc/systemd/system/cryptoboned.service ] ; then # re-install symlinks systemctl disable cryptoboned rm -f /etc/systemd/system/cryptoboned.service 2> /dev/null systemctl enable cryptoboned fi 8. %files /etc/init.d/cryptoboned -> %{_sysconfdir}/init.d/cryptoboned /usr/share/man/man8/cryptoboned.8.gz -> %{_mandir}/man8/cryptoboned.8.gz I think that's enough for now! -- 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