https://bugzilla.redhat.com/show_bug.cgi?id=1906980 Robert-André Mauchin 🐧 <zebob.m@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Whiteboard|NotReady | --- Comment #5 from Robert-André Mauchin 🐧 <zebob.m@xxxxxxxxx> --- (In reply to Egor Artemov from comment #1) > I'm not in the packagers group however has some recommendations: > Thanks for your informative review. > 1) Build fails on aarch64, s390x, and ppc64le. > It looks like you should list these architectures using the `ExcludeArch:` > tag, as per packaging guidelines. > > Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=57335109 > Guideline: > https://docs.fedoraproject.org/en-US/packaging-guidelines/ > #_architecture_build_failures > I added a ExclusiveArch since ppc and s390 are not supposed to be supported. > 2) Since the package provides only a static library, it will be better to > package it as a `highway-devel` package with virtual '-static' provide as > per > packaging guidelines. > See: > https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static- > libraries Indeed, fixed. > 3) It seems documentation includes large PDF files like slides from the > presentation, > examples, and project skeleton > might be it is better to move them into the separate `-doc` package? > Done. > 4) It seems you shouldn't install all headers, that upstream installs. > As per > https://github.com/google/highway/blob/master/g3doc/quick_reference.md > The only public headers that should be installed are (with dependencies): > hwy/highway.h > hwy/base.h > hwy/foreach_target.h > hwy/aligned_allocator.h > hwy/cache_control.h > hwy/targets.h > hwy/ops/x86_128-inl.h > hwy/ops/x86_256-inl.h > hwy/ops/x86_512-inl.h > hwy/ops/arm_neon-inl.h > hwy/ops/wasm_128-inl.h > hwy/ops/scalar-inl.h > hwy/ops/shared-inl.h > hwy/set_macros-inl.h > > But looks like it is better to fire an issue upstream to fix > CMakeFiles.txt > I removed the non useful headers and filed a bug upstream. > 5) It seems it is better to move commands for tests running into `%check` > section. Done. -- 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 _______________________________________________ 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 on the list, report it: https://pagure.io/fedora-infrastructure
