https://bugzilla.redhat.com/show_bug.cgi?id=2237084 Fabio Valentini <decathorpe@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends On| |2237104 (rust-markdown), | |2237101 (rust-include_dir), | |2237103 (rust-log-panics) Flags| |fedora-review? Assignee|nobody@xxxxxxxxxxxxxxxxx |decathorpe@xxxxxxxxx --- Comment #7 from Fabio Valentini <decathorpe@xxxxxxxxx> --- Thanks! I have a few quick comments before I do a full review: ------------------------------------------------------------ The upstream tarball bundles a copy of wxWidgets. It needs to be removed in %prep: %prep ... # drop vendored wxWidgets sources rm -r espanso-modulo/vendor ------------------------------------------------------------ > %cargo_build -- --package={espanso,espanso-clipboard,espanso-config,espanso-detect,espanso-engine,espanso-info,espanso-inject,espanso-ipc,espanso-kvs,espanso-mac-utils,espanso-match,espanso-migrate,espanso-modulo,espanso-package,espanso-path,espanso-render,espanso-ui} --bin espanso --features="default" --bin espanso-wayland --features="wayland" This should not be necessary, and I also think it doesn't do what you actually want. This should be enough to achieve the same thing: %cargo_build -f wayland ------------------------------------------------------------ Similarly, for %cargo_test, the following should be equivalent to what you're doing manually: %cargo_test -- -- --skip ipc_multiple_clients ------------------------------------------------------------ If you want to actually have distinct License tags for espanso and espanso-wayland, you also need to call %cargo_license / %cargo_license_summary with different arguments for both crates: # for "default" %{cargo_license_summary} %{cargo_license} > LICENSE.dependencies # for "wayland" %{cargo_license_summary -f wayland} %{cargo_license -f wayland} > LICENSE.dependencies.wayland Otherwise one or the other will not be accurate. In general, you should apply the same "-a" / "-f" flags to *all* %cargo_* macros (except %cargo_prep) to avoid inconsistent results or weird errors. ------------------------------------------------------------ Package also doesn't built yet due to missing dependencies: - include_dir (pending review) - log-panics (approved, pending unretirement) - markdown (pending review) There's also "yaml-rust-terzi", which is a fork of the "yaml-rust" crate. The code for it will need to be bundled in this package, and the dependency replaced with a `path = "./path/to/yaml-rust"`-style dependency). Referenced Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=2237101 [Bug 2237101] Review Request: rust-include_dir - Embed the contents of a directory in your binary https://bugzilla.redhat.com/show_bug.cgi?id=2237103 [Bug 2237103] Review Request: rust-log-panics - Panic hook which logs panic messages rather than printing them https://bugzilla.redhat.com/show_bug.cgi?id=2237104 [Bug 2237104] Review Request: rust-markdown - Native Rust library for parsing Markdown and (outputting HTML) -- 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=2237084 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202237084%23c7 _______________________________________________ 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
