https://bugzilla.redhat.com/show_bug.cgi?id=1377762 Carl George <carl@george.computer> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@xxxxxxxxxxxxxxxxx |carl@george.computer Flags| |fedora-review? --- Comment #10 from Carl George <carl@george.computer> --- I should be able to review this for you. I haven't run fedora-review on this yet, but just reading the spec file I noticed a few things that can be improved. Change the Source0 URL to the recommended format [0]: -Source0: https://github.com/savoirfairelinux/%{name}/archive/%{version}.tar.gz +Source0: https://github.com/savoirfairelinux/%{name}/archive/%{version}/%{name}-%{version}.tar.gz For libraries detected via pkg-config [1], use the pkgconfig() format [2]: -BuildRequires: msgpack-devel -BuildRequires: gnutls-devel -BuildRequires: nettle-devel -BuildRequires: libargon2-devel +BuildRequires: pkgconfig(msgpack) +BuildRequires: pkgconfig(gnutls) +BuildRequires: pkgconfig(nettle) +BuildRequires: pkgconfig(libargon2) The upstream wiki [3] says the Cython build requirement is for the optional Python bindings, but I don't see those listed in your %files. Also the upstream setup.py.in file [4] indicates this is for Python 3, not 2, so you need to build require python3-Cython. Wrap the lines of the description at 80 characters [5]. Don't use the Group tag [6]. The first line of the autogen.sh script [7] is to run `git submodule update --init`. You don't have git as a build requirement, so the build logs have the error `BUILDSTDERR: ./autogen.sh: line 1: git: command not found`. But more importantly, packages aren't allowed to download from the internet during the build. The submodule is for argon2, which you already build require, so skip autogen.sh and just run autoreconf directly. Use %make_build and %make_install [8]. Remove the ldconfig scriptlets, RPM takes care of this automatically now [9]. Unless you intend to also package this for EPEL, in which case use %ldconfig_scriptlets (which is a no-op on Fedora). Remove `%{!?_licensedir:%global license %doc}`, %license is defined in all Fedora and EPEL branches now. Your %changelog needs at least one item for the 2018 entry, such as `- Latest upstream`. Alternatively you can just delete the 2016 line and use the initial packaging item for the 2018 entry. Upstream has a test suite, try to add a %check section and run it [10]. [0]: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_git_tags [1]: https://github.com/savoirfairelinux/opendht/blob/1.8.2/configure.ac#L128-L130 [2]: https://docs.fedoraproject.org/en-US/packaging-guidelines/PkgConfigBuildRequires/ [3]: https://github.com/savoirfairelinux/opendht/wiki/Build-the-library [4]: https://github.com/savoirfairelinux/opendht/blob/1.8.2/python/setup.py.in#L41-L45 [5]: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_summary_and_description [6]: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_tags_and_sections [7]: https://github.com/savoirfairelinux/opendht/blob/1.8.2/autogen.sh#L1 [8]: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_parallel_make [9]: https://fedoraproject.org/wiki/Changes/Removing_ldconfig_scriptlets [10]: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_test_suites -- 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://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
