[Bug 817306] Review Request: libircclient - C library to create IRC clients

[bugzilla at redhat.com]

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=817306

--- Comment #5 from Thomas Spura <tomspur at fedoraproject.org> 2012-05-01 15:10:27 EDT ---
(In reply to comment #4)
> Thanks again for the review.
> 
> I uploaded updated spec and srpm on top of the
> previous ones.

Please always bump the release, add a changelog entry and describe what you
have changed and publish a new src.rpm with the new release entry.
Then link to the new spec/src.rpm in the new comment, so you don't need to
search where the spec/src.rpm is etc. I promise, it will make sense and is not
just a nitpick ;)

> Yes, my FAS account is pcpa.

Thanks.

> I opened a generic bug report about inconsistency
> with static library packages and guidelines at
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=817888

I'm afraid, there needs to be a bug for all packages, that don't comply with
the guidelines as Bill writes there... :(

> About the package, I actually implemented the
> suggestion I made in the upstream bug report,
> and attached it to the report, so that now it
> by default creates a shared library.

Great.

> I also rebuilt the megaglest package to ensure
> it still works correctly. Actually, with a shared
> libircclient, The megaglest patch
> megaglest-3.6.0.3-openssl.patch is no longer
> required, as now libircclient "pulls" the equivalent
> of "-lssl -lcrypto"

Great^2 :)


Continuing the review:
- cocoa is still there
- license correct now
- It doesn't compile, as you seem to have an old libirc library installed, when
building, but when not having it installed, the new build library isn't found:
gcc -o spammer spammer.o -L../src/ -lircclient -lpthread -lssl -lcrypto  -lnsl
/usr/bin/ld: cannot find -lircclient
collect2: error: ld returned 1 exit status

Doing a "ln -s libircclient.so.0 src/libircclient.so" helps to build it.
--> A "ln -s libircclient.so.$(MAJOR) libircclient.so" is missing in the patch.

- rpmlint issues:

$ rpmlint /home/tomspur/rpmbuild/SRPMS/libircclient-1.6-1.fc17.src.rpm
/home/tomspur/rpmbuild/RPMS/x86_64/libircclient-1.6-1.fc17.x86_64.rpm
/home/tomspur/rpmbuild/RPMS/x86_64/libircclient-devel-1.6-1.fc17.x86_64.rpm
/home/tomspur/rpmbuild/RPMS/x86_64/libircclient-debuginfo-1.6-1.fc17.x86_64.rpm
libircclient.src: I: enchant-dictionary-not-found en_US
libircclient.src:30: W: mixed-use-of-spaces-and-tabs (spaces: line 30, tab:
line 1)
libircclient.x86_64: W: no-documentation
libircclient-devel.x86_64: W: summary-not-capitalized C libircclient
development files.
libircclient-devel.x86_64: W: summary-ended-with-dot C libircclient development
files.
libircclient-devel.x86_64: W: no-documentation
libircclient-debuginfo.x86_64: E: empty-debuginfo-package
4 packages and 0 specfiles checked; 1 errors, 5 warnings.

Please correct mixed-use-of-spaces-and-tabs, summary-not-capitalized,
summary-ended-with-dot.

The empty-debuginfo-packages is normally a sign, when a package didn't apply
the optflags, but that seems to be the case.
In this case, you are building the library with "-s", which strips the library.
Please remove that.
More information about that here:
http://fedoraproject.org/wiki/Packaging:Debuginfo

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.