Bug 437694 - Review Request: bip - IRC Bouncer
Review Request: bip - IRC Bouncer
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Tom "spot" Callaway
Fedora Extras Quality Assurance
:
: 442219 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-03-16 11:32 EDT by Lorenzo Villani
Modified: 2014-10-13 18:59 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-05-20 12:54:07 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tcallawa: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Lorenzo Villani 2008-03-16 11:32:10 EDT
Spec URL: http://rpm.binaryhelix.org/specs/network/bip.spec
SRPM URL: http://rpm.binaryhelix.org/bip-0.7.0-1.fc8.src.rpm
Description: Bip is an IRC proxy, which means it keeps connected to your preferred IRC
servers, can store the logs for you, and even send them back to your IRC
client(s) upon connection.
You may want to use bip to keep your logfiles (in a unique format and on a
unique computer) whatever your client is, when you connect from multiple
workstations, or when you simply want to have a playback of what was said
while you were away.

Notes:
- rpmlint on built package gives no errors
- builds cleanly on mock (on my system, at least)

Questions:
- I have another package under review, don't know if I have to add FE-NEEDSPONSOR blocker also here or not
Comment 1 Gianluca Varisco 2008-04-11 08:27:12 EDT
I can't sponsor you, but as reviewer I just tried to build it and it seems OK.
Comment 2 manuel wolfshant 2008-04-11 11:01:47 EDT
Gianluca. verify that it builds is one step, but for a proper review you should
follow http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
Comment 3 manuel wolfshant 2008-04-11 11:02:05 EDT
Gianluca. verifying that it builds is one step, but for a proper review you
should follow http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
Comment 4 Jeffrey C. Ollie 2008-04-11 15:42:15 EDT
I haven't tried bip out yet, but version 0.7.2 of bip is out...
Comment 5 Lorenzo Villani 2008-04-12 07:35:30 EDT
Spec URL: http://rpm.binaryhelix.org/specs/network/bip.spec
SRPM URL: http://rpm.binaryhelix.org/bip-0.7.2-1.fc8.src.rpm

Bumped to the new version
Comment 6 manuel wolfshant 2008-04-12 18:18:07 EDT
*** Bug 442219 has been marked as a duplicate of this bug. ***
Comment 7 Lorenzo Villani 2008-04-13 15:29:09 EDT
Spec URL: http://rpm.binaryhelix.org/specs/network/bip.spec
SRPM URL: http://rpm.binaryhelix.org/bip-0.7.2-2.fc8.src.rpm

* Sun Apr 13 2008 Lorenzo Villani <lvillani@binaryhelix.net> - 0.7.2-2
- Added AUTHORS, ChangeLog, COPYING, INSTALL, README, TODO to docdir
- added --enable-ssl to %configure, just to make sure that bip is built
  with SSL support using OpenSSL
Comment 8 manuel wolfshant 2008-04-13 18:17:31 EDT
Please do not package INSTALL. It is only useful for people building from source. 
Comment 9 Lorenzo Villani 2008-04-14 05:41:00 EDT
Spec URL: http://rpm.binaryhelix.org/specs/network/bip.spec
SRPM URL: http://rpm.binaryhelix.org/bip-0.7.2-3.fc8.src.rpm

* Mon Apr 14 2008 Lorenzo Villani <lvillani@binaryhelix.net> - 0.7.2-3
- Removed INSTALL from %doc
Comment 10 Lorenzo Villani 2008-04-22 09:31:28 EDT
FYI:
Darryl L. Pierce (dpierce@redhat.com) wants to co-maintain this package and I
agreed to co-maintain the package with him.
Comment 11 Tom "spot" Callaway 2008-04-29 17:07:49 EDT
One very minor thing, instead of:

rm -rf %{buildroot}/usr/share/doc/bip

Please use: 

rm -rf $RPM_BUILD_ROOT%{_defaultdocdir}/bip

Why? 

1. You need to be consistent with $RPM_BUILD_ROOT or %{buildroot}. You've
already used $RPM_BUILD_ROOT above this line.
2. _defaultdocdir == /usr/share/doc
Comment 12 Tom "spot" Callaway 2008-04-29 17:12:54 EDT
Also, this isn't building with the Fedora OPTFLAGS. Making this change will
resolve it:

Before:
make %{?_smp_mflags}

After:
make %{?_smp_mflags} CFLAGS="$RPM_OPT_FLAGS"
Comment 13 Tom "spot" Callaway 2008-04-29 17:16:39 EDT
Oh yes, rpmlint says:

bip.src:52: W: macro-in-%changelog doc
bip.src:56: W: macro-in-%changelog configure
bip.x86_64: W: file-not-utf8 /usr/share/doc/bip-0.7.2/ChangeLog

Please resolve these three warnings (hint, if you have any %macros in changelog
entries, change them to %%macros). To fix the ChangeLog, do this in %prep:

iconv -f iso-8859-1 -t utf-8 -o ChangeLog{.utf8,}
mv ChangeLog{.utf8,}

Show me a new SPEC/SRPM with all of these items resolved and I'll finish the review.
Comment 14 Lorenzo Villani 2008-04-30 08:29:26 EDT
Spec URL: http://rpm.binaryhelix.org/specs/network/bip.spec
SRPM URL: http://rpm.binaryhelix.org/bip-0.7.2-4.fc9.src.rpm


* Wed Apr 30 2008 Lorenzo Villani <lvillani@binaryhelix.net> - 0.7.2-4
- Convert ChangeLog to utf-8 in prep
- Ensure that package is compiled using RPM_OPT_FLAGS
- Make usage of RPM_BUILD_ROOT consistent
- Removed macros from ChangeLog (bad mistake)
Comment 15 Tom "spot" Callaway 2008-04-30 08:47:54 EDT
I don't think you need that explicit Requires: openssl, since the package picks
it up automatically (see libcrypto.so.7 and libssl.so.7):

Requires: libc.so.6()(64bit) libc.so.6(GLIBC_2.2.5)(64bit)
libc.so.6(GLIBC_2.3)(64bit) libc.so.6(GLIBC_2.3.4)(64bit)
libc.so.6(GLIBC_2.4)(64bit) libcrypto.so.7()(64bit) libssl.so.7()(64bit) openssl
rtld(GNU_HASH)

Also, the license should be GPLv2+, not GPLv2. Minor distinction, but the code
all has:

 * This program is free software; you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation; either version 2 of the License, or
 * (at your option) any later version.
 * See the file "COPYING" for the exact licensing terms.

Make those two corrections and I'll approve.

REVIEW
=======

- rpmlint checks return nothing.
- package meets naming guidelines
- package meets packaging guidelines
- license (GPLv2+) OK, text in %doc
- spec file legible, in am. english
- source matches upstream (94c1b44bd49c65dde5d006b2df236449a53a1aa9)
- package compiles on devel (x86_64)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file 
Comment 16 Lorenzo Villani 2008-04-30 10:41:16 EDT
Spec URL: http://rpm.binaryhelix.org/specs/network/bip.spec
SRPM URL: http://rpm.binaryhelix.org/bip-0.7.2-4.fc9.src.rpm


* Wed Apr 30 2008 Lorenzo Villani <lvillani@binaryhelix.net> - 0.7.2-5
- Corrected License field
- Removed openssl from Requires
Comment 17 Tom "spot" Callaway 2008-04-30 10:50:38 EDT
Approved. :) I will also sponsor you.
Comment 18 Lorenzo Villani 2008-05-01 08:57:15 EDT
New Package CVS Request
=======================
Package Name: bip
Short Description: IRC Bouncer
Owners: arbiter
Branches: F-9 EL-5
InitialCC: 
Cvsextras Commits: yes
Comment 19 Kevin Fenzi 2008-05-01 11:31:13 EDT
cvs done.
Comment 20 Lorenzo Villani 2008-05-01 16:00:16 EDT
Package Change Request
======================
Package Name: bip
Updated Fedora Owners: arbiter,mcpierce
Comment 21 Kevin Fenzi 2008-05-02 11:49:09 EDT
I don't see mcpierce in the cvsextras group. Have they been sponsored?
Can you double check the name?
Comment 22 Darryl L. Pierce 2008-05-02 13:29:41 EDT
Kevin: I was sponsored by T Callaway.
Comment 23 Jason Tibbitts 2008-05-02 13:45:11 EDT
It doesn't look that way.  Or at least, the process is not complete because
currently mcpierce is not yet in the cvsextras group.

https://admin.fedoraproject.org/accounts/user/view/mcpierce
Comment 24 Lorenzo Villani 2008-05-08 08:42:09 EDT
Package Change Request
======================
Package Name: bip
Updated Fedora Owners: arbiter,mcpierce
Comment 25 Kevin Fenzi 2008-05-08 13:36:49 EDT
cvs done.
Comment 26 Brian Lane 2014-10-08 16:57:18 EDT
Package Change Request
======================
Package Name: bip
New Branches: epel7
Owners: bcl mcpierce mmahut
Comment 27 Kevin Fenzi 2014-10-13 18:59:09 EDT
Git done (by process-git-requests).

Note You need to log in before you can comment on or make changes to this bug.