[Bug 2241245] Review Request: rust-gst-plugin-gtk4 - GStreamer GTK 4 Sink element and Paintable widget

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux