https://bugzilla.redhat.com/show_bug.cgi?id=2233278 --- Comment #12 from Kalev Lember <klember@xxxxxxxxxx> --- Thanks a lot for the review! I'll fix some of your notes before importing and continue tomorrow. > 1. It looks like there are no tests (neither defined in meson or in cargo), so you could drop the %check section. I deliberately added the %check section so that if tests appear in the future, they'd automatically get run :) I don't think I'd notice if/when they get added upstream otherwise. %meson_test behaviour where it is no-op when no tests are defined nicely makes it possible. > 2. You're missing the "%bcond_with(out) check". Not defining this can mess with cargo macros, especially %cargo_generate_buildrequires. > In this case, it doesn't matter, because there aren't any test dependencies or tests, but keep this in mind. In almost all cases, if you're using the RPM macros for cargo, you will want to define either "%bcond_without check" or "%bcond_with check". Ohh, let me fix that. Thanks! > 3. Upstream released v0.1-beta.3 alongside v0.1.0-beta.3 of glycin-utils and glycin crates. Please update as soon as possible to match the other two components. Right, it depends on updating rust-librsvg that I didn't want to do today, but I'll sort this out tomorrow. > 4. Please replace "BR: rust-packaging" with "BR: cargo-rpm-macros". The former no longer exists and is only provided by the latter for backwards compatibility. OK! > 5. The dependency on pkgconfig(gtk4) seems to be a noop? It's declared as a dependency in meson.build, but not assigned to an object or used anywhere. Not sure what's up with that. Yes, not sure. I'll investigate. -- 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=2233278 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202233278%23c12 _______________________________________________ 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
