Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: libircclient-qt - Cross-platform IRC client library written with Qt 4
https://bugzilla.redhat.com/show_bug.cgi?id=707617
Summary: Review Request: libircclient-qt - Cross-platform IRC client library written with Qt 4 Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: jkaluza@redhat.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://jkaluza.fedorapeople.org/libircclient-qt.spec SRPM URL: http://jkaluza.fedorapeople.org/libircclient-qt-0.5.0-1.fc14.src.rpm Description: Cross-platform IRC client library written with Qt 4
$ rpmlint libircclient-qt-0.5.0-1.fc14.src.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings.
$ rpmlint libirc* 3 packages and 0 specfiles checked; 0 errors, 0 warnings.
I'm adding this package, because new version of Spectrum will need it (optionally) for IRC support.
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=707617
Michael Schwendt mschwendt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |mschwendt@gmail.com AssignedTo|nobody@fedoraproject.org |mschwendt@gmail.com Flag| |fedora-review?
--- Comment #1 from Michael Schwendt mschwendt@gmail.com 2011-07-30 06:49:24 EDT --- * "spectool -g libircclient-qt.spec" fails downloading the tarball. Visiting the web page download area, it seems the correct Source0 location is without the leading 'www.' - download then succeeds: https://bitbucket.org/jpnurmi/libircclient-qt/downloads/%%7Bname%7D-src-%%7B...
%package devel Group: System Environment/Libraries
As long as we keep filling this Group tag, the correct group for library -devel packages is: Development/Libraries
%package devel License: LGPLv2+ URL: http://www.bitbucket.org/jpnurmi/libircclient-qt
Repeating License and URL tags here is not needed. So, unless you insist on maintaining them in two places, better delete them here.
%package devel Requires: %{name} = %{version}-%{release}
%{?_isa} is missing: https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package
%files devel ... %{_includedir}/ircclient-qt/Irc ... %{_libdir}/qt4/mkspecs/features/libircclient-qt-config.prf %{_libdir}/qt4/mkspecs/features/libircclient-qt.prf
Which package owns %{_includedir}/ircclient-qt/ and the several directories below %_libdir? https://fedoraproject.org/wiki/Packaging:UnownedDirectories
Also, since several essential headers, such as "irc.h" include Qt headers, there's a "Requires: qt-devel" missing in the -devel package.
%files devel ... %doc COPYING AUTHORS README CHANGELOG
These %doc files are duplicates of the ones installed by the base package. As a result, this only clutters up /usr/share/doc/ where two directories for the libircclient-qt packages would be created.
* What about packaging the 'doc' directory with its HTML documentation?
* Build log warns about deprecated features being enabled and adds many warnings about individual deprecated items. What's the story here? The 'INSTALL' file comments on that, but doesn't answer when the "deprecated backwards compatible functionality" could be needed.
* Build log: compilation is non-verbose. One cannot see the used compiler flags, for example. This is because the QMake project file adds CONFIG += silent.
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=707617
--- Comment #2 from Jan Kaluža jkaluza@redhat.com 2011-08-03 03:28:00 EDT --- Thanks for taking this.
Spec URL: http://jkaluza.fedorapeople.org/libircclient-qt.spec SRPM: http://jkaluza.fedorapeople.org/libircclient-qt-0.5.0-2.fc15.src.rpm Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=3248944
(In reply to comment #1)
- "spectool -g libircclient-qt.spec" fails downloading the tarball.
Should be fixed now.
As long as we keep filling this Group tag, the correct group for library -devel packages is: Development/Libraries
Fixed too.
Repeating License and URL tags here is not needed. So, unless you insist on maintaining them in two places, better delete them here.
Fixed.
%{?_isa} is missing:
Fixed.
%{_includedir}/ircclient-qt/Irc
This directory is now owned by libircclient-qt
%{_libdir}/qt4/mkspecs/features/libircclient-qt-config.prf %{_libdir}/qt4/mkspecs/features/libircclient-qt.prf
Those are owned by qt-devel. I've added qt-devel as Requires for -devel package.
Also, since several essential headers, such as "irc.h" include Qt headers, there's a "Requires: qt-devel" missing in the -devel package.
Fixed.
These %doc files are duplicates of the ones installed by the base package.
Removed from -devel.
- What about packaging the 'doc' directory with its HTML documentation?
Added.
- Build log warns about deprecated features being enabled and adds many
warnings about individual deprecated items. What's the story here? The 'INSTALL' file comments on that, but doesn't answer when the "deprecated backwards compatible functionality" could be needed.
During make, moc_FILENAME.cpp meta files are created by Qt, which does some magic to make Qt signals working (generates unique signals signatures and so on). This meta files also contain signals handlers, so the signals are called there. If some signals are deprecated, it will trigger that warning. This could be fixed by disabling deprecated signals completely, but I think this should be done by upstream first.
- Build log: compilation is non-verbose. One cannot see the used compiler
flags, for example. This is because the QMake project file adds CONFIG += silent.
I'm commenting that line using sed now.
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=707617
Michael Schwendt mschwendt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #3 from Michael Schwendt mschwendt@gmail.com 2011-08-06 06:58:56 EDT --- APPROVED
Except for one thing the rpmdev-diff output reveals:
-%post -p /sbin/ldconfig +%post -p /bin/echo +1
-%postun -p /sbin/ldconfig +%postun -p /bin/echo +1
Probably you've tested something there, but it makes no sense to keep that change. ;)
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=707617
--- Comment #4 from Jan Kaluža jkaluza@redhat.com 2011-08-08 04:34:48 EDT --- Ah, you're right. I've been testing one rpm behaviour there (which resulted in rpm bugzilla) and forgot to remove it. I will remove it when committing to repo.
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=707617
Jan Kaluža jkaluza@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #5 from Jan Kaluža jkaluza@redhat.com 2011-08-08 04:49:31 EDT --- New Package SCM Request ======================= Package Name: libircclient-qt Short Description: Cross-platform IRC client library written with Qt 4 Owners: jkaluza Branches: f15 f16 InitialCC:
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=707617
--- Comment #6 from Jon Ciesla limb@jcomserv.net 2011-08-08 06:05:01 EDT --- Git done (by process-git-requests).
package-review@lists.fedoraproject.org