https://bugzilla.redhat.com/show_bug.cgi?id=1818670 Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |zbyszek@xxxxxxxxx Doc Type|--- |If docs needed, set a value --- Comment #2 from Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> --- The description is supposed to go in the spec file primarily. (Usually the one in the review request is just a copy. What counts is the one in the file.) Please wrap it to 80 columns when copying into the file. Sources should be extracted in %prep, not in %build. The scripts to add 'sensu' user and group need to be a bit different: please use the scriptlet from https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/#_dynamic_allocation Users cannot be removed from the system, because this generally cannot be done safely. Also, do not remove non-cache data on package removal. The scriptlets to define the user should be present in just one subpackage, and the other subpackage should Require it. If both packages are fully independent and both need the user, add a comment to the spec file about that. The %postun systemd scriptlets are missing, see https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scriptlets You can drop full paths like in /usr/sbin/userdel. Both /usr/bin and /usr/sbin are guaranteed to be in the path. There is also no gain in using macros like %{__install}. They serve no purpose whatsoever and make the spec file much harder to read. (There is a guidelines to use macros for some _directories_, because they change occasionally, but binary names don't change.) Systemd unit files go to /usr/lib/systemd/system, not in etc, see https://www.freedesktop.org/software/systemd/man/systemd.unit.html#Description. /var/run/sensu/ → /run/sensu. It also needs %ghost. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_run BuildRequires: systemd → systemd-rpm-macros should be enough, it is much more lightweight. > Group: Development/Languages Not necessary, please remove. Requires(post): systemd Requires(preun): systemd Requires(postun): systemd → not necessary, should be removed. Following https://docs.fedoraproject.org/en-US/packaging-guidelines/Golang/#_dependencies, please add: BuildRequires: go-rpm-macros. The big question at the end: would it be possible to unbundle any of the go dependencies, i.e. to use some packages from Fedora. It seems that doing that for all 70+ modules is infeasible, but maybe a partial unbundling would work. I don't know about go to have an informed opinion. If everything is bundled, this needs a comment in the spec file and justification. -- 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 To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
