https://bugzilla.redhat.com/show_bug.cgi?id=1636111 --- Comment #12 from mati86dl@xxxxxxxxx --- Hi, Except debug, I think it's all done. Spec URL: https://services.delellis.com.ar/data/rpmbuild/SPECS/glyr.spec SRPM URL: https://services.delellis.com.ar/data/rpmbuild/SRPMS/glyr-1.0.10-7.20180824git618c418e.fc28.src.rpm Inline comments to fixes.. > please remove "Group: " tag, see https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections Done. > - the "License:" field is not correct anymore. see "COPYING", the license has > been changed to LGPLv3. D'Oh!. Never see the licence change. Done. > - instead of "BuildRequires: glib2-devel >= 2.10", might want to > * "BuildRequires: pkgconfig(glib-2.0) > 2.10" and > * "BuildRequires: pkgconfig(gthread-2.0)" > see https://fedoraproject.org/wiki/Packaging:PkgConfigBuildRequires Done. I knew these recommendations, but I took them as optional, and continued with other priorities. > - nit, might want to patch the `CMakeLists.txt`, as its version number is still > 1.0.9. please 'grep GLYR_VERSION_MICRO' in the source tree for more details. Done.. Add a patch to fix the version. It was a recurring problem with the versions in this project. :disapointed: > - in %description section, it'd be better to add hypyen between "easy to use" so > it looks like "easy-to-use". because it helps user to digest it. see also: > https://en.oxforddictionaries.com/punctuation/hyphen Done.. Thanks fo the tip. > - could you justify why debug_package is disabled? If remove them just result on: > Empty %files file /home/matias/Desarrollo/rpmbuild/BUILD/glyr/debugsourcefiles.list ..and honestly I can not find how to fix it. You can help me.? :disapointed: > - could you link 0001-use-lastfm-getinfo-instead-getimages.patch to upstream bugs/comments/lists, > or justify it in the header of the patch? Done. Just remove that. They were internal tests to try to solve a bug in libclastfm. Both libclastfm an libglyr in Pragha use queries in parallel with different private keys, then try to differentiate them. > - libglyr comes with a test suite, shall we have %check for exercising it and for making sure all > tests pass? at least, capi could be tested, i guess. Well, check_api and check_dbc its ok, but check_opt fail.. So, change '%{cmake}' to '%{cmake} -DTEST=true' to enable build test and append these: > > %check > > bin/check_api > > bin/check_dbc > > # This check fails so ignore that. > > # bin/check_opt It's ok? > - "Requires: libcurl" does not look right. please leave it to rpm, it is able to figure out > the runtime dependency introduced by linked shared libraries. You are right. Also sqlite3. > - If your application is a C or C++ application you must list a > BuildRequires against gcc, gcc-c++ or clang. > Note: No gcc, gcc-c++ or clang found in BuildRequires > See: https://fedoraproject.org/wiki/Packaging:C_and_C%2B%2B Done.. Is half strange to specify a particular compiler, but done. BuildRequires: gcc I await your comments, Regards -- 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
