https://bugzilla.redhat.com/show_bug.cgi?id=2274028 --- Comment #5 from Dorinda <dbassey@xxxxxxxxxx> --- (In reply to blinxen from comment #2) > I took a quick peek and have the following comments: > > * Both patches ("ignore-pw-server-test.patch" and > "build-fix-for-alsa-test.patch") simply add the `ignore` attribute to some > tests. > Therefore I see them as redundant and would just skip them in the `%check` > section like you did with `result::tests::async_seq_panic`. > * The patch for `i386` should be called "build-fix-for-i686", since this is > what we actually build in Fedora. > `i386` is only used in copr. > * Is the `i386` patch forwarded to upstream? I think it might be useful to > them too. > * Why do all packages have "BuildArch: noarch"? > * The man page should not be added by using a patch. Use `Source` and copy > the man page in the `%install` section. Also this can / should be forwarded > to upstream. > * The comment "# FIXME: paste output of %%cargo_license_summary here" should > be removed since you wrote down the output of `%cargo_license_summary`. > * Please add the output of "%cargo_license_summary" as a comment in the spec > file above `License`. We do this for all Rust packages and it makes it > easier to double check the license tags. > * Is there a specific reason for including the binary file and the man page > in the devel subpackage? I think this was added by mistake since the binary > package does not contain the man page. > > > I think there's some packaging issue with devel packages for the backends. I see for example: > > I would have expected different content there. > > The output looks correct to me. What content would you have expected? fixed: removed patches to ignore tests by skipping them in the `%check` fixed: renamed The patch for `i386` to "build-fix-for-i686" fixed: removed the The comment "# FIXME" added a new comment indicating the License fixed: put binary file and man page in the proper location (main package) > * Why do all packages have "BuildArch: noarch"? the package dependencies specfiles specifies "BuildArch: noarch", I think it'll be better to clarify with the maintainer of those packages. > * The man page should not be added by using a patch. Use `Source` and copy > the man page in the `%install` section. Also this can / should be forwarded > to upstream. for now I let the man page to be a patch, I would send this upstream ASAP. -- 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=2274028 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202274028%23c5 -- _______________________________________________ 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
