https://bugzilla.redhat.com/show_bug.cgi?id=2316473
Cristian Le <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |[email protected] --- Comment #12 from Cristian Le <[email protected]> --- A few things to point out: - Do not override `%_libdir`. For arch dependent projects, libraries must be installed under `/usr/lib64` which is the default - The project is by definition a C-only project. Please patch in `LANGUAGES C` under `project` [1] and remove the `gcc-c++` dependency - `ctest` were not executed. Please add `-DBUILD_UNITTESTS=ON` to make sure they are compiled and run (navigate to the `Executing(%check)` section to see if they are actually executed) - Do not wildcard Licenses. It is important to keep track of license changes and that would obfuscate things - Please use macros for the snapshot details (commit date and hash). You can use `%forgemeta` macros [2] or just manually define `%commit` and `%snapshot_date` [3] - I prefer not to include `%{?epoch:%{epoch}:}` until it is actually needed, but some may have better reasoning if/when it's needed early on - Preference: use a more constrained glob pattern for the `%_includedir`. Something like `*.h` is fine imo, but others prefer to list all files explicitly - Preference: some (me included) prefer to separate sections by 2 blank lines and order them in the execution order: `%prep` -> `%generate_buildrequires` -> (`%conf` only available since 4.18 ->) `%build` -> `%install` -> `%check` (also `%files` and `%changelong` even though they are not similar scriplets) Furthermore could you contact upstream about packaging issues, and you can ping me on those as a person of contact for CMake stuff: - Use `CMakePackageConfigHelpers` macros [4] and include the `safestringlibVersion.cmake` file - Do not manually add compilation flags (it interferes with our optimization flags), instead move them to presets and only run them in the CI - Bump the minimum cmake version (if the minimum is too low, recent CMake changes and imporvements are not applied) - Consider migrating the includes into dedicated subfolder [1]: https://github.com/intel/safestringlib/blob/fd159bf8c37a2a0b7ba328766f3c96d9aa075f27/CMakeLists.txt#L7-L10 [2]: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_using_forges_hosted_revision_control [3]: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_git_hosting_services [4]: https://cmake.org/cmake/help/latest/module/CMakePackageConfigHelpers.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=2316473 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202316473%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
