|Summary:||Review Request: pidgin-otr - OTR messaging for pidgin (was gaim-otr)|
|Product:||[Fedora] Fedora||Reporter:||Paul Wouters <pwouters>|
|Component:||Package Review||Assignee:||Kevin Fenzi <kevin>|
|Status:||CLOSED RAWHIDE||QA Contact:||Fedora Extras Quality Assurance <extras-qa>|
|Fixed In Version:||Doc Type:||Bug Fix|
|Doc Text:||Story Points:||---|
|Last Closed:||2007-08-28 04:03:55 UTC||Type:||---|
|oVirt Team:||---||RHEL 7.3 requirements from Atomic Host:|
Description Paul Wouters 2007-08-02 05:07:07 UTC
Spec URL: ftp://ftp.xelerance.com/pidgin-otr/pidgin-otr.spec SRPM URL: ftp://ftp.xelerance.com/pidgin-otr/pidgin-otr-3.1.0-1.fc7.src.rpm Description: This is a Pidgin plugin which implements Off-the-Record (OTR) Messaging. It is known to work (at least) under the Linux and Windows versions of Pidgin. This package used to be called gaim-otr, also packaged by me. The intention is to obsolete gaim-otr. pidgin-otr requires libotr-3.1.0, which was only just pushed into devel/
Comment 1 Kevin Fenzi 2007-08-05 20:04:12 UTC
I'd be happy to review this package. Look for a full review in a bit.
Comment 2 Kevin Fenzi 2007-08-05 21:50:51 UTC
OK - Package meets naming and packaging guidelines OK - Spec file matches base package name. OK - Spec has consistant macro usage. OK - Meets Packaging Guidelines. See below - License OK - License field in spec matches OK - License file included in package OK - Spec in American English OK - Spec is legible. OK - Sources match upstream md5sum: f4ca8bc228069616d4c8fa9288af6bb5 pidgin-otr-3.1.0.tar.gz f4ca8bc228069616d4c8fa9288af6bb5 pidgin-otr-3.1.0.tar.gz.1 See below - BuildRequires correct OK - Spec handles locales/find_lang OK - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - Package has correct buildroot OK - Package is code or permissible content. OK - Packages %doc files don't affect runtime. OK - Package has rm -rf RPM_BUILD_ROOT at top of %install OK - .la files are removed. OK - Package compiles and builds on at least one arch. OK - Package has no duplicate files in %files. OK - Package doesn't own any directories other packages own. OK - Package owns all the directories it creates. See below - No rpmlint output. OK - final provides and requires are sane SHOULD Items: OK - Should build in mock. OK - Should build on all supported archs OK - Should function as described. OK - Should have sane scriptlets. OK - Should have dist tag OK - Should package latest version Issues: 1. Per the new License guidelines I think the License: tag here should be GPLv2 (GPL version 2 only). 2. Your Provides and Obsoletes for gaim-otr don't look right to me. See: http://fedoraproject.org/wiki/Packaging/NamingGuidelines#head-3cfc1ea19d28975faad9d56f70a6ae55661d3c3d I think you should have: Provides: gaim-otr = 3.0.1-0.7.20060712cvs.fc7 Obsoletes: gaim-otr < 3.0.1-0.6.20060712cvs.fc7 3. Doesn't build here under mock. I get: checking for XML::Parser... configure: error: XML::Parser perl module is required for intltool error: Bad exit status from /var/tmp/rpm-tmp.1879 (%build) If I add a "BuildRequires: perl(XML::Parser)" it gets further and then blows up with: make: Entering directory `/builddir/build/BUILD/pidgin-otr-3.1.0/po' file=`echo es | sed 's,.*/,,'`.gmo \ && rm -f $file && -o $file es.po file=`echo fr | sed 's,.*/,,'`.gmo \ && rm -f $file && -o $file fr.po /bin/sh: line 1: -o: command not found make: *** [es.gmo] Error 127 make: *** Waiting for unfinished jobs.... /bin/sh: line 1: -o: command not found make: *** Waiting for unfinished jobs.... /bin/sh: line 1: -o: command not found make: *** [fr.gmo] Error 127 make: Leaving directory `/builddir/build/BUILD/pidgin-otr-3.1.0/po' make: *** [all-recursive] Error 1 make: Leaving directory `/builddir/build/BUILD/pidgin-otr-3.1.0' make: *** [all] Error 2 error: Bad exit status from /var/tmp/rpm-tmp.47112 (%build) Needs to also have "BuildRequires: gettext" That gets it building. 4. rpmlint says: W: pidgin-otr invalid-license GPL W: pidgin-otr invalid-license GPL W: pidgin-otr unversioned-explicit-obsoletes gaim-otr W: pidgin-otr-debuginfo invalid-license GPL Would be fixed by the issue 1 and issue 2 being fixed.
Comment 3 Paul Wouters 2007-08-06 15:08:03 UTC
Thanks for the review! I have fixed the license, the unversioned obsolete, and the buildrequires How did you build pidgin-otr in mock, with libotr not being available? Or did you run mock on a "devel" system? I don't have a devel system, so I couldn't run it easilly with mock, and I'm waiting for pidgin-otr to be available before bumping both libotr and pidgin-otr into f7 and fc6. Spec URL: ftp://ftp.xelerance.com/pidgin-otr/pidgin-otr.spec SRPM URL: ftp://ftp.xelerance.com/pidgin-otr/pidgin-otr-3.1.0-2.fc7.src.rpm
Comment 4 Paul Wouters 2007-08-06 17:22:57 UTC
ok, confirmed it with a koji scratch build.
Comment 5 Kevin Fenzi 2007-08-07 02:29:52 UTC
Yes, I build in mock for devel by default. I also often build f7 and fc6. In this case I just looked at the devel output. Your Provides is still using the version of this package instead of just slightly larger than the last gaim-otr that it's replacing. Thats not a big deal in this case, but the guidelines have that rule so that if the package you are replacing ever did come back you could just add it back and it would be ok, instead of needing to be a higher version than this package. Since gaim-otr is unlikely ever to return, that can be ignored in this case. I see no further blockers, so this package is APPROVED. Don't forget to close this once it's been imported and built. Also, do consider doing some reviews of other waiting packages to help spread out the review load.
Comment 6 Paul Wouters 2007-08-07 02:49:51 UTC
Actually, the latest release of gaim-otr is 3.0.1-0.6.20060712cvs So as per http://fedoraproject.org/wiki/Packaging/NamingGuidelines#head-3cfc1ea19d28975faad9d56f70a6ae55661d3c3d I bumped the release one to 3.0.1-0.7.20060712cvs for the Obsoletes: Thanks for the review. I will pick up a few other reviews in return. New Package CVS Request ======================= Package Name: pidgin-otr Short Description: OTR messaging for pidgin (was gaim-otr) Owners: email@example.com Branches: F-7 (since pidgin is not in <= FC-6)
Comment 7 Paul Wouters 2007-08-07 02:59:08 UTC
Actually there is pidgin for FC-6, so I'm asking for a branche there too. New Package CVS Request ======================= Package Name: pidgin-otr Short Description: OTR messaging for pidgin (was gaim-otr) Owners: firstname.lastname@example.org Branches: FC-6, F-7
Comment 8 Kevin Fenzi 2007-08-07 03:26:16 UTC
Comment 9 Kevin Fenzi 2007-08-28 04:03:55 UTC
This package appears to have been imported and built. Closing this review request now.
Comment 10 Paul Wouters 2015-01-31 17:31:08 UTC
Package Change Request ====================== Package Name: pidgin-otr New Branches: epel7 Owners: pwouters
Comment 11 Gwyn Ciesla 2015-02-01 15:41:39 UTC
Git done (by process-git-requests).