Bug 240324 - Review Request: telepathy-idle - IRC connection manager for Telepathy
Review Request: telepathy-idle - IRC connection manager for Telepathy
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Kevin Fenzi
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-05-16 10:46 EDT by Brian Pepple
Modified: 2007-11-30 17:12 EST (History)
1 user (show)

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


Attachments (Terms of Use)
Mock build log for telepathy-idle (57.28 KB, text/plain)
2007-05-16 10:47 EDT, Brian Pepple
no flags Details

  None (edit)
Description Brian Pepple 2007-05-16 10:46:23 EDT
Spec URL: http://www.ceplovi.cz/matej/progs/rpms/telepathy-idle.spec
SRPM URL: http://www.ceplovi.cz/matej/progs/rpms/telepathy-idle-0.0.5-1.src.rpm
Description: A full-featured IRC connection manager for the Telepathy project.

Note there is a newer version of telepathy-idle, but it depends on telepathy-glib which isn't in Fedora yet since that package has some issues that need to be worked out before submission.
Comment 1 Brian Pepple 2007-05-16 10:47:12 EDT
Created attachment 154829 [details]
Mock build log for telepathy-idle
Comment 2 Šimon Lukašík 2007-05-18 12:52:41 EDT
Hello; Firstly this is my first attempt to review so something can be
convoluted. In this case correct me please.

Version 0.0.5 isn't the newest. Here is 0.1.0:
http://telepathy.freedesktop.org/releases/telepathy-idle/telepathy-idle-0.1.0.tar.gz
Otherwise, check for telepathy-idle-0.0.5-1: (looks fine)
-----------------------------------------------------------------------------
* MUST: rpmlint must be run on every package. The output should be posted in the
review.
        OK, clear
* MUST: The package must be named according to the Package Naming Guidelines.
        OK
* MUST: The spec file name must match the base package %{name}, in the format
%{name}.spec unless your package has an exemption on Package Naming Guidelines.
        OK
* MUST: The package must meet the Packaging Guidelines.
        OK, I think so.
* MUST: The package must be licensed with an open-source compatible license and
meet other legal requirements as defined in the legal section of Packaging
Guidelines.
        OK; LGPL
* MUST: The License field in the package spec file must match the actual license.
        OK; LGPL 2.1, February 1999
* MUST: If (and only if) the source package includes the text of the license(s)
in its own file, then that file, containing the text of the license(s) for the
package must be included in %doc.
        OK; COPYING
* MUST: The spec file must be written in American English.
        OK
* MUST: The spec file for the package MUST be legible.
        OK
* MUST: The sources used to build the package must match the upstream source, as
provided in the spec URL.
        OK; 4e782df6d3858b4886d3a23be475c17e
* MUST: The package must successfully compile and build into binary rpms on at
least one supported architecture.
        OK; i386
* MUST: If the package does not successfully compile, build or work on an
architecture, then those architectures should be listed in the spec in ExcludeArch.
        OK; no ExcluseArch meanwhile; No notice from upstream.
* MUST: All build dependencies must be listed in BuildRequires.
        OK
* MUST: The spec file MUST handle locales properly.
        OK
* MUST: Every binary RPM package which stores shared library files (not just
symlinks) in any of the dynamic linker's default paths, must call ldconfig in
%post and %postun. If the package has multiple subpackages with libraries, each
subpackage should also have a %post/%postun section that calls /sbin/ldconfig.
        OK
* MUST: If the package is designed to be relocatable, the packager must state
this fact in the request for review, along with the rationalization for
relocation of that specific package. Without this, use of Prefix: /usr is
considered a blocker.
        OK
* MUST: A package must own all directories that it creates.
        OK
* MUST: A package must not contain any duplicate files in the %files listing.
        OK
* MUST: Permissions on files must be set properly. Executables should be set
with executable permissions, for example. Every %files section must include a
%defattr(...) line.
        OK
* MUST: Each package must have a %clean section, which contains rm -rf
%{buildroot} (or $RPM_BUILD_ROOT).
        OK
* MUST: Each package must consistently use macros, as described in the macros
section of Packaging Guidelines.
        OK
* MUST: The package must contain code, or permissable content. This is described
in detail in the code vs. content section of Packaging Guidelines.
        OK
* MUST: Large documentation files should go in a -doc subpackage.
        OK;
* MUST: If a package includes something as %doc, it must not affect the runtime
of the application.
        OK
* MUST: Header files must be in a -devel package.
        OK; There is no header files in telepathy-idle package.
* MUST: Static libraries must be in a -static package.
        Ok
* MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'
        OK
* MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1),
then library files that end in .so (without suffix) must go in a -devel package.
        OK
* MUST: In the vast majority of cases, devel packages must require the base
package using a fully versioned dependency.
        OK; Not devel
* MUST: Packages must NOT contain any .la libtool archives, these should be
removed in the spec.
        OK
* MUST: Packages containing GUI applications must include a %{name}.desktop
file, and that file must be properly installed with desktop-file-install in the
%install section.
        OK; No GUI.
* MUST: Packages must not own files or directories already owned by other packages.
        OK
* MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot}
(or $RPM_BUILD_ROOT).
        OK
* MUST: All filenames in rpm packages must be valid UTF-8.
        OK; simple ASCII
---------------------------------------------------------------------------------
Comment 3 Matěj Cepl 2007-06-01 18:55:50 EDT
Looks absolutely terrific; and upgraded package is on
http://www.ceplovi.cz/matej/progs/rpms/telepathy-idle-0.1.0-1.fc7.src.rpm

APPROVED!!!
Comment 4 Matěj Cepl 2007-06-01 19:09:05 EDT
New Package CVS Request
=======================
Package Name: telepathy-idle
Short Description: IRC connection manager for Telepathy
Owners: bdpepple@ameritech.net, mcepl@redhat.com
Branches: FC-6 FC-7
Comment 5 Jason Tibbitts 2007-06-01 21:27:12 EDT
This ticket is quite confusing.

It's not assigned to anybody.  Who reviewed it?

It looks like the person who approved it is the person who is updating the
packages, which is, well, rather odd.
Comment 6 Brian Pepple 2007-06-01 21:39:34 EDT
The package that Matej references in comment #3 can't go into Fedora yet, since
it depends on a package (telepathy-glib) which isn't in Fedora, as noted in my
initial comment #1.  Matej was hosting my original spec & srpm, since I retired
my old web server.
Comment 7 Tom "spot" Callaway 2007-06-02 14:46:28 EDT
Dropping the CVS request. You can't review your own package.
Comment 8 Brian Pepple 2007-06-02 15:23:19 EDT
(In reply to comment #7)
> Dropping the CVS request. You can't review your own package.

Umm, I didn't review the package.
Comment 9 manuel wolfshant 2007-06-02 17:08:40 EDT
Tom, from the bug's activity it looks like
- Šimon Lukašík did a review, but avoided to assign it to himself or approve it
because he considers himself not experienced enough
- Matej Cepl approved it, but forgot to assign the package to himself

I guess that the proper continuation is (assuming Simon's review is correct and
Matej did verify it)
- Matej should assign the package to himself and approve it (hence fedora-review+)
- Brian should afterwards make the CVS request and set fedora-cvs?
Comment 10 Brian Pepple 2007-06-02 21:24:55 EDT
Reposting this for review since there has been so much confusion:

Spec URL: http://spotbox.dyn.dhs.org/telepathy-idle.spec
SRPM URL: http://spotbox.dyn.dhs.org/telepathy-idle-0.0.5-1.src.rpm
Description: A full-featured IRC connection manager for the Telepathy project.

**Note** there is a newer version of telepathy-idle, but it depends on
telepathy-glib which isn't in Fedora yet since that package has some issues that
need to be worked out before submission.
Comment 11 Kevin Fenzi 2007-06-04 23:00:21 EDT
I'd be happy to do a formal review of this and stave off any confusion...

Look for a full review in a few. 
Comment 12 Kevin Fenzi 2007-06-04 23:20:31 EDT
OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
OK - License (LGPL)
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:
4e782df6d3858b4886d3a23be475c17e  telepathy-idle-0.0.5.tar.gz
4e782df6d3858b4886d3a23be475c17e  telepathy-idle-0.0.5.tar.gz.1
See below - BuildRequires correct
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 - 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 have dist tag
See below - Should package latest version

Issues:

1. Non blocker/Cosmetic:
rpmlint says:

W: telepathy-idle mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 13)

Fix if you like.

2. possible missing buildrequires? From the build.log:
./configure: line 20691: GTK_DOC_CHECK: command not found
BuildRequires: gtk-doc?
Comment 13 Brian Pepple 2007-06-04 23:37:13 EDT
(In reply to comment #12)
> 2. possible missing buildrequires? From the build.log:
> ./configure: line 20691: GTK_DOC_CHECK: command not found
> BuildRequires: gtk-doc?

The tarball contains no documentation to create with gtk-doc, so adding it as a
BR is fairly pointless right now.



Comment 14 Kevin Fenzi 2007-06-04 23:42:12 EDT
Ah yes, sorry about that. A closer look does indeed show no docs for gtk-doc. ;( 

ok. I see no blockers left, so this package is APPROVED. 
Don't forget to close this review once this package is imported and built. 
Comment 15 Brian Pepple 2007-06-04 23:48:28 EDT
No worries.  Thanks for the review, Kevin!

New Package CVS Request
=======================
Package Name: telepathy-idle
Short Description: IRC connection manager for Telepathy
Owners: bdpepple@ameritech.net
Branches: FC-6 F-7
InitialCC:
Comment 16 Tom "spot" Callaway 2007-06-05 11:15:09 EDT
not doing cvs. confused beyond belief.

Just kidding. CVS done.
Comment 17 Brian Pepple 2007-06-05 14:07:36 EDT
thanks, spot. ;)

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