https://bugzilla.redhat.com/show_bug.cgi?id=2241245 --- Comment #8 from Kalev Lember <klember@xxxxxxxxxx> --- (In reply to Fabio Valentini from comment #7) > Some minor issues: > > 1. The License tag for the "source" package in line 26 should only contain > the license of the project itself (MPL-2.0). > The licenses for statically linked Rust dependencies should only go into the > License tag for the gstreamer1-plugin-gtk4 subpackage. Ohh, oops, I had added it to the wrong package. Fixed! > 2. You can use the %cargo_cbuild and %cargo_install macros instead of using > the "private" %__cargo macro to manually call these commands. > I recently added them to the cargo-c package and haven't yet had the > opportunity to switch the gst-plugin-reqwest package to them. Nice, I didn't know about the new macros! That makes it much cleaner. > 3. Why do you use the `-a` (--all-features) flag for > %cargo_generate_buildrequires / %cargo_build / %cargo_install / %cargo_test? > It doesn't look like the shared object is built with "--all-features", so I > don't see why it should be necessary. Or did you *want* to build the plugin > with all features enabled? Then the "--all-features" is missing from cbuild > / cinstall calls *and* from the %cargo_license_summary / %cargo_license > macro calls. I was working around a build failure without really understanding what --all-features does. Looks like updated 0.11.0 builds fine without it so I just went ahead and removed -a from everywhere :) I'll ask you if the issue should resurface. Not entirely sure what was going on there. > 4. It would be great if you could add virtual Provides for the upstream name > to the plugin subpackage as well, similar to what we've done in > gst-plugin-reqwest. The naming convention for GStreamer plugins in Fedora is > rather weird since it doesn't match upstream names at all, so providing the > upstream name can prevent some confusion here, I think. Sure, added. * Fri Sep 29 2023 Kalev Lember <klember@xxxxxxxxxx> - 0.11.0-2 - Package review fixes (#2241245) - Fix source package license to only contain the license of the project itself - Add virtual provides for gst-plugin-gtk4 to gstreamer1-plugin-gtk4 subpackage - Use cargo_cbuild and cargo_cinstall macros - Drop inadvertent --all-features flag Spec URL: https://kalev.fedorapeople.org/rust-gst-plugin-gtk4.spec SRPM URL: https://kalev.fedorapeople.org/rust-gst-plugin-gtk4-0.11.0-2.fc40.src.rpm Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=106892373 Diff with the latest changes if it makes it easier for you to review changes: https://paste.centos.org/view/97e99661 -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=2241245 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202241245%23c8 _______________________________________________ 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
