Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: hiredis - A C client library for redis
https://bugzilla.redhat.com/show_bug.cgi?id=698067
Summary: Review Request: hiredis - A C client library for redis Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: shakthimaan@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora Story Points: ---
Spec URL: http://shakthimaan.fedorapeople.org/SPECS/hiredis.spec SRPM URL: http://shakthimaan.fedorapeople.org/SRPMS/hiredis-0.9.2-1.fc14.src.rpm Description: A minimalistic C client library for redis
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
--- Comment #1 from Shakthi Kannan shakthimaan@gmail.com 2011-04-20 01:02:13 EDT --- $ rpmlint hiredis.spec hiredis.spec: W: invalid-url Source0: antirez-hiredis-v0.9.2-0-g0fbfa45.zip 0 packages and 1 specfiles checked; 0 errors, 1 warnings.
$ rpmlint ../SRPMS/hiredis-0.9.2-1.fc14.src.rpm hiredis.src: W: spelling-error Summary(en_US) minimalistic -> minimalist, Minimalist, minimalism hiredis.src: W: spelling-error %description -l en_US minimalistic -> minimalist, Minimalist, minimalism hiredis.src: W: invalid-url Source0: antirez-hiredis-v0.9.2-0-g0fbfa45.zip 1 packages and 0 specfiles checked; 0 errors, 3 warnings.
$ rpmlint ../RPMS/i686/hiredis-0.9.2-1.fc14.i686.rpm hiredis.i686: W: spelling-error Summary(en_US) minimalistic -> minimalist, Minimalist, minimalism hiredis.i686: W: spelling-error %description -l en_US minimalistic -> minimalist, Minimalist, minimalism hiredis.i686: W: shared-lib-calls-exit /usr/lib/libhiredis.so.1.0 exit@GLIBC_2.0 hiredis.i686: W: no-manual-page-for-binary hiredis-example hiredis.i686: W: no-manual-page-for-binary hiredis-test 1 packages and 0 specfiles checked; 0 errors, 5 warnings.
$ rpmlint ../RPMS/i686/hiredis-devel-0.9.2-1.fc14.i686.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings.
Will inform upstream to remove exit() calls from code. Successful Koji builds for F14, F15 and EL6:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3012740 http://koji.fedoraproject.org/koji/taskinfo?taskID=3012741 http://koji.fedoraproject.org/koji/taskinfo?taskID=3012744
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
Volker Fröhlich volker27@gmx.at changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |volker27@gmx.at
--- Comment #2 from Volker Fröhlich volker27@gmx.at 2011-05-07 10:08:31 EDT --- Some comments:
There is a version 0.10.0 release upstream. Please update!
You can drop your second patch and instead write:
make install PREFIX=%{buildroot}/%{_prefix} INSTALL_LIB=%{buildroot}/%{_libdir}
That's also replacing /usr with %{_prefix}. Please also place comments on your patches do in the spec file. You can use the name macro on some occasions, e. g. when installing.
I think you can make up a better description, given the description on the homepage. The description of the devel package even seems wrong to me, because the devel package certainly does not contain libraries to use a database.
redis should be a BuildRequires -- not a Requires.
I don't know how you managed the EPEL 6 build. Currently there is only a redis in testing, as far as I can see.
The compiler flags are not as requires by Fedora. See http://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags
Please install the TODO file as documentation.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
--- Comment #3 from Shakthi Kannan shakthimaan@gmail.com 2011-05-10 03:11:36 EDT --- Thanks for your comments. Updated:
SPEC: http://shakthimaan.fedorapeople.org/SPECS/hiredis.spec SRPM: http://shakthimaan.fedorapeople.org/SRPMS/hiredis-0.10.0-1.gitdf203bc328.fc1...
* The 0.10.0 fails to build, so have updated to hiredis-0.10.0-1.gitdf203bc328.
* Updated use of:
make install PREFIX=%{buildroot}/%{_prefix} INSTALL_LIB=%{buildroot}/%{_libdir}
For 64-bit, we need to override LIBRARY_PATH.
* redis is not required to build hiredis, but, only to use it. Hence, have used Requires.
* Added TODO file as documentation.
$ rpmlint hiredis.spec hiredis.spec: W: invalid-url Source0: hiredis.gitdf203bc328.tar.bz2 0 packages and 1 specfiles checked; 0 errors, 1 warnings.
$ rpmlint hiredis-0.10.0-1.gitdf203bc328.fc14.src.rpm hiredis.src: W: spelling-error Summary(en_US) minimalistic -> minimalist, Minimalist, minimalism hiredis.src: W: spelling-error %description -l en_US minimalistic -> minimalist, Minimalist, minimalism hiredis.src: W: invalid-url Source0: hiredis.gitdf203bc328.tar.bz2 1 packages and 0 specfiles checked; 0 errors, 3 warnings.
$ rpmlint hiredis-0.10.0-1.gitdf203bc328.fc14.i686.rpm hiredis.i686: W: spelling-error Summary(en_US) minimalistic -> minimalist, Minimalist, minimalism hiredis.i686: W: spelling-error %description -l en_US minimalistic -> minimalist, Minimalist, minimalism hiredis.i686: W: no-manual-page-for-binary hiredis-example hiredis.i686: W: no-manual-page-for-binary hiredis-test 1 packages and 0 specfiles checked; 0 errors, 4 warnings.
$ rpmlint hiredis-devel-0.10.0-1.gitdf203bc328.fc14.i686.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings.
Successful Koji builds for F-14, F-15, and EL-6:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3062235 http://koji.fedoraproject.org/koji/taskinfo?taskID=3062238 http://koji.fedoraproject.org/koji/taskinfo?taskID=3062241
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
--- Comment #4 from Volker Fröhlich volker27@gmx.at 2011-05-10 17:13:29 EDT --- Actually the upstream 0.10.0 builds just fine. Please use the .tar.gz from upstream, if it is smaller in size.
Please see the spec below and apply the changes to your file. The changes basically are a corrected devel sub-package description, use of optimization flags and a simpler install path part.
http://www.geofrogger.net/review/hiredis.spec
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
--- Comment #5 from Shakthi Kannan shakthimaan@gmail.com 2011-05-10 22:41:17 EDT --- Thanks for the changes. Updated:
SPEC: http://shakthimaan.fedorapeople.org/SPECS/hiredis.spec SRPM: http://shakthimaan.fedorapeople.org/SRPMS/hiredis-0.10.0-2.fc14.src.rpm
$ rpmlint hiredis.spec hiredis.spec: W: invalid-url Source0: antirez-hiredis-v0.10.0-3-gdf203bc.tar.gz 0 packages and 1 specfiles checked; 0 errors, 1 warnings.
$ rpmlint hiredis-0.10.0-2.fc14.src.rpm hiredis.src: W: spelling-error Summary(en_US) minimalistic -> minimalist, Minimalist, minimalism hiredis.src: W: spelling-error %description -l en_US minimalistic -> minimalist, Minimalist, minimalism hiredis.src: W: invalid-url URL: https://github.com/antirez/hiredis <urlopen error [Errno 111] Connection refused> hiredis.src: W: invalid-url Source0: antirez-hiredis-v0.10.0-3-gdf203bc.tar.gz 1 packages and 0 specfiles checked; 0 errors, 4 warnings.
$ rpmlint hiredis-0.10.0-2.fc14.i686.rpm hiredis.i686: W: spelling-error Summary(en_US) minimalistic -> minimalist, Minimalist, minimalism hiredis.i686: W: spelling-error %description -l en_US minimalistic -> minimalist, Minimalist, minimalism hiredis.i686: W: invalid-url URL: https://github.com/antirez/hiredis <urlopen error [Errno 111] Connection refused> hiredis.i686: W: no-manual-page-for-binary hiredis-example hiredis.i686: W: no-manual-page-for-binary hiredis-test 1 packages and 0 specfiles checked; 0 errors, 5 warnings.
$ rpmlint hiredis-devel-0.10.0-2.fc14.i686.rpm hiredis-devel.i686: W: invalid-url URL: https://github.com/antirez/hiredis <urlopen error [Errno 111] Connection refused> 1 packages and 0 specfiles checked; 0 errors, 1 warnings.
Successful Koji builds for F-14, F-15, EL-6:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3064370 http://koji.fedoraproject.org/koji/taskinfo?taskID=3064385 http://koji.fedoraproject.org/koji/taskinfo?taskID=3064388
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
Volker Fröhlich volker27@gmx.at changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nobody@fedoraproject.org |volker27@gmx.at Flag| |fedora-review?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
Volker Fröhlich volker27@gmx.at changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #6 from Volker Fröhlich volker27@gmx.at 2011-05-14 15:38:20 EDT --- It's common to leave out the full stop in the changelog, as it is more like a list of things.
By the way: INSTALL_LIB seems to be useless. Must be some leftover.
The summary says, the package was a "client". I think this should be "client library".
I encourage you to create a sub-package for the test and example file, to keep the library package small, or delete it at all, if it is not useful.
FYI: http://groups.google.com/group/linux.debian.devel.mentors/browse_thread/thre... ----------------------------------------
Review:
[+] Good [-] Needs work [0] Does not apply
MUST: =====
[+] rpmlint: [makerpm@fedora14 adapters]$ rpmlint ~/rpmbuild/SRPMS/hiredis-0.10.0-2.fc14.src.rpm ~/rpmbuild/RPMS/x86_64/hiredis-*0.10.0-2.fc14.x86_64.rpm hiredis.src: W: spelling-error Summary(en_US) minimalistic -> minimalist, Minimalist, minimalism hiredis.src: W: spelling-error %description -l en_US minimalistic -> minimalist, Minimalist, minimalism hiredis.src: W: invalid-url Source0: antirez-hiredis-v0.10.0-3-gdf203bc.tar.gz hiredis.x86_64: W: spelling-error Summary(en_US) minimalistic -> minimalist, Minimalist, minimalism hiredis.x86_64: W: spelling-error %description -l en_US minimalistic -> minimalist, Minimalist, minimalism hiredis.x86_64: W: no-manual-page-for-binary hiredis-example hiredis.x86_64: W: no-manual-page-for-binary hiredis-test 4 packages and 0 specfiles checked; 0 errors, 7 warnings.
[+] Naming according to the Package Naming Guidelines [+] Spec file matches base package name [+] Packaging guidelines met [+] License approved for Fedora [+] License field in spec matches code [+] License file included, if source package includes it [+] Spec in American English [+] Spec is legible [+] Sources match upstream md5sum: b32b930e5e1ee007594c1056c3ff1c0e [+] Compiles and builds into binary RPMs on at least one primary architecture [0] ExcludeArch is specified and commented [0] Locales are handled correctly [+] All build dependencies listed [+] Calls ldconfig for its shared libraries [+] No bundled system libraries [0] Stated as relocatable package [+] Owns all its directories or requires package that does [+] No file listing duplicates [+] File permissions correct [+] Consistent use of macros [+] Code or permissible content [0] Large documentation in -doc subpackage [+] No runtime dependency of files listed as %doc [+] Header files in -devel subpackage [0] Static files in -static subpackage [+] Library files without suffix in -devel subpackage [+] Devel-package requires base package [0] No .la libtool archives [0] GUI application includes properly installed %{name}.desktop file [+] No files or directories owned, that other packages own [+] Filenames in packages are UTF-8
SHOULD: =======
[0] Query upstream if no license text is included [+] Package builds in mock: Tried epel-6-x86_64 and fedora-rawhide-i386 [?] Package works as described -- Haven't tried [0] Scriptlets are sane, if used [0] Subpackages other than -devel should require base package (versioned) [0] pkgconfig files in -devel subpackage [0] Dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself [0] Contain man pages, where they make sense
-------- APPROVED --------
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
--- Comment #7 from Shakthi Kannan shakthimaan@gmail.com 2011-05-16 01:28:31 EDT --- Thanks again for the review and comments. Updated to use 'client library' in Summary and removed INSTALL_LIB.
SPEC: http://shakthimaan.fedorapeople.org/SPECS/hiredis.spec SRPM: http://shakthimaan.fedorapeople.org/SRPMS/hiredis-0.10.0-3.fc14.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
Shakthi Kannan shakthimaan@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #8 from Shakthi Kannan shakthimaan@gmail.com 2011-05-16 01:30:55 EDT --- New Package SCM Request ======================= Package Name: hiredis Short Description: A minimalistic C client library for Redis Owners: shakthimaan Branches: F-14 F-15 EL-6 InitialCC: shakthimaan
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
--- Comment #9 from Jason Tibbitts tibbs@math.uh.edu 2011-05-18 18:19:52 EDT --- Git done (by process-git-requests).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |MODIFIED
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
--- Comment #10 from Fedora Update System updates@fedoraproject.org 2011-05-19 01:01:15 EDT --- hiredis-0.10.0-3.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/hiredis-0.10.0-3.fc14
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
--- Comment #11 from Fedora Update System updates@fedoraproject.org 2011-05-19 01:01:28 EDT --- hiredis-0.10.0-3.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/hiredis-0.10.0-3.el6
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
--- Comment #12 from Fedora Update System updates@fedoraproject.org 2011-05-19 01:01:36 EDT --- hiredis-0.10.0-3.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/hiredis-0.10.0-3.fc15
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #13 from Fedora Update System updates@fedoraproject.org 2011-05-19 17:59:17 EDT --- hiredis-0.10.0-3.fc14 has been pushed to the Fedora 14 testing repository.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
--- Comment #14 from Fedora Update System updates@fedoraproject.org 2011-05-24 23:04:54 EDT --- hiredis-0.10.0-3.fc15 has been pushed to the Fedora 15 stable repository.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Fixed In Version| |hiredis-0.10.0-3.fc15 Resolution| |ERRATA Last Closed| |2011-05-24 23:04:59
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
--- Comment #15 from Fedora Update System updates@fedoraproject.org 2011-05-27 16:20:27 EDT --- hiredis-0.10.0-3.fc14 has been pushed to the Fedora 14 stable repository.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|hiredis-0.10.0-3.fc15 |hiredis-0.10.0-3.fc14
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
--- Comment #16 from Fedora Update System updates@fedoraproject.org 2011-06-06 17:33:13 EDT --- hiredis-0.10.0-3.el6 has been pushed to the Fedora EPEL 6 stable repository.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=698067
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|hiredis-0.10.0-3.fc14 |hiredis-0.10.0-3.el6
package-review@lists.fedoraproject.org