https://bugzilla.redhat.com/show_bug.cgi?id=1728381 Cole Robinson <crobinso@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review? --- Comment #7 from Cole Robinson <crobinso@xxxxxxxxxx> --- FWIW this packages is quite similar to driverctl which is already in Fedora. driverctl review bug: https://bugzilla.redhat.com/show_bug.cgi?id=1372670 Trimmed output from fedora-review: - Sources used to build the package match the upstream source, as provided in the spec URL. See: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/ The source URL looks acceptable per the docs, but looks like the archive in the SRPM is not the same content as github generates. Please fix that - systemd_post is invoked in %post, systemd_preun in %preun, and systemd_postun in %postun for Systemd service files. See: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scriptlets The guidelines don't list anything about systemd parameterized units (ones with @ in the name), which is what mdevctl is using. driverctl doesn't use the macros, but openvpn _does_ use them for its parameterized units, like this: %post %systemd_post openvpn-client@\*.service %systemd_post openvpn-server@\*.service %preun %systemd_preun openvpn-client@\*.service %systemd_preun openvpn-server@\*.service %postun %systemd_postun_with_restart openvpn-client@\*.service %systemd_postun_with_restart openvpn-server@\*.service %systemd_postun_with_restart openvpn@\*.service I don't really understand enough about how all this works to say whether this package should be using those macros. aw I'll leave it up to you to decide. Can always be fixed after import if necessary - Changelog in prescribed format. Changelog lines should be individually prefixed with '-' and contain a version string at the end. Your changelog there looks more like it should be a NEWS.md file which you can ship as %doc. Using that is better for upstream too IMO because other distros won't want a .spec file to be the canonical release notes. For Fedora spec the changelog should be the package version history so all of those entries should be trimmed except the most recent one basically. mdevctl.noarch: W: empty-%post mdevctl.noarch: W: empty-%postun Looks like on all current Fedora versions, udev_rules_update is an empty macro, because file triggers are used to make udev update itself. https://github.com/systemd/systemd/commit/32a00a9c097cf04ec2b0fcbf9b73eba188318424 So you can remove those sections entirely. If you need those for another distro like rhel, please wrap them in a conditional to make it clear they don't apply to Fedora -- 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
