https://bugzilla.redhat.com/show_bug.cgi?id=2233278



--- Comment #12 from Kalev Lember <[email protected]> ---
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 -- [email protected]
To unsubscribe send an email to [email protected]
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/[email protected]
Do not reply to spam, report it: 
https://pagure.io/fedora-infrastructure/new_issue

Reply via email to