https://bugzilla.redhat.com/show_bug.cgi?id=2239566 Carl George 🤠 <carl@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review? --- Comment #2 from Carl George 🤠 <carl@xxxxxxxxxx> --- Ultimately FPC decided that this package is not exempt, so I'll proceed with the review. On a first pass I noticed the following issues: ================================================================================ There is no URL tag, resulting in an rpmlint warning. On a related note, there are quite a few source files in this package. Would it be possible to establish an upstream repository somewhere like GitHub or GitLab that can be used to obtain tarballs of the source files? That upstream could also determine the versions. Right now it's not clear where the version of 1.0.42 is coming from. No other file in the SRPM besides the spec file has that string. ================================================================================ /usr/lib/dracut/modules.d/99kdumpbase/monitor_dd_progress has some inconsistencies with other scripts in that directory. It is missing the .sh extension and is not executable. The latter is resulting in an rpmlint error. If none of these scripts should be executable, then the rpmlint error can be cleared by removing the shebang lines. This file also is triggering an rpmlint warning for "potential-bashisms". I'm not sure if this is something that can be resolved or not, but please look into it. The checkbashisms CLI tool is flagging the usage of `echo -e` on line 24. ================================================================================ rpmlint is also reporting an error that this architecture-specific package does not contain any architecture-specific binaries. Should it be marked as noarch? ================================================================================ Consider converting instances of $RPM_BUILD_ROOT to %{buildroot}. This is not required, as the guidelines only say that you need to use one or the other consistently, but the latter is considered more modern and is more common in Fedora spec files. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_using_buildroot_and_optflags_vs_rpm_build_root_and_rpm_opt_flags ================================================================================ I noticed the remove_prefix helper function in the spec file. Is that really necessary? It's more characters than just using the filename directly in the install commands. Fedora spec files aim for legibility, and I think using a helper function that requires more characters to invoke than just being explicit with the installed filename is less legible. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_spec_legibility ================================================================================ fedora-review and rpmlint are flagging an issue with /usr/lib/udev/rules.d (and recursively applying to 98-kexec.rules in that directory) being marked as %config. %config should only be used on files in /etc. ================================================================================ I noticed there are systemd service files and a target file installed to /usr/lib/dracut/modules.d/99kdumpbase. Should those be installed to the regular systemd directories, or do they need to be here? ================================================================================ This package has a license tag of GPL-2.0-only, but /usr/lib/dracut/modules.d/99kdumpbase/kdump-capture.service and /usr/lib/dracut/modules.d/99kdumpbase/kdump-emergency.target have headers that indicate they are licensed as LGPL-2.1-or-later. Please refer to the license guidelines to set the license tag appropriately. https://docs.fedoraproject.org/en-US/legal/license-field/#_conjunctive_and_licensing ================================================================================ fedora-review is also flagging this as uninstallable, but upon closer inspection this appears to be because it needs makedumpfile present first. We may want to finish that review first for simplicity. -- 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 https://bugzilla.redhat.com/show_bug.cgi?id=2239566 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202239566%23c2 -- _______________________________________________ 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 Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
