Bug 541355 - Review Request: python-couchdb - A Python library for working with CouchDB
Summary: Review Request: python-couchdb - A Python library for working with CouchDB
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Terje Røsten
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-11-25 17:03 UTC by Sebastian Dziallas
Modified: 2009-12-17 01:04 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-12-17 01:04:09 UTC
Type: ---
Embargoed:
terje.rosten: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Sebastian Dziallas 2009-11-25 17:03:32 UTC
Spec URL: http://sdz.fedorapeople.org/rpmbuild/python-couchdb.spec
SRPM URL: http://sdz.fedorapeople.org/rpmbuild/python-couchdb-0.6-1.fc12.src.rpm

Koji Scratch Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1830708

[sebastian@localhost SRPMS]$ rpmlint python-couchdb-0.6-1.fc12.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
[sebastian@localhost noarch]$ rpmlint python-couchdb-0.6-1.fc12.noarch.rpm 
python-couchdb.noarch: E: explicit-lib-dependency python-httplib2
1 packages and 0 specfiles checked; 1 errors, 0 warnings.

The latter one should be fine, since it's a python package. This package is also a dependency for desktopcouch.

Comment 1 Terje Røsten 2009-11-25 20:14:25 UTC
Tiny comments, you might want to more explicit in the %files section.

This is a couchdb client? Is then couchdb really needed on the client host?

Comment 2 Sebastian Dziallas 2009-11-25 20:53:42 UTC
Agreed on the %files section, will do so with the next release.

It's, from what I understand, a library for accessing couchdb through python. So couchdb would be required (the author's page says so in the requirements: http://code.google.com/p/couchdb-python/).

I was actually packaging it because it's a dependency for desktopcouch, which has so goal to integrate couchdb into desktop applications.

Comment 3 Terje Røsten 2009-11-29 11:48:38 UTC
More comments:

- the srpm linked don't work, however the koji srpm builds fine
- please include the doc/ dir
- do chmod more elegante e.g.
  find  $RPM_BUILD_ROOT/%{python_sitelib}/couchdb -name \*.py -print0 | xargs --null chmod 0644
- switch to %{__python} in build and install

Fix these and I will do a formal review.

Comment 4 Jef Spaleta 2009-12-14 20:26:34 UTC
Hey Terje, 

If Sebastian is cool with it... I'll make some of the requested edits and push you a new spec file and put myself up as co-maintainer.

Opinion as to whether doc/  should be in a docs subpackage or in the main package?

Comment 5 Jef Spaleta 2009-12-14 21:46:34 UTC
Terje,

Here's the link to an updated specfile and packages. 
http://fedorapeople.org/~jspaleta/python-couchdb/

 Koji isn't cooperating right now due to the intrastructure work going on i think but I built this locally on my F10 system without issue.

I've made a devel subpackage for the api documentation as per the suggestion in the packaging guidelines.

I've patched the python files under site-packages directory to remove the shebang as they are not meant to be executable and are not in an executable directory.  This has already been filed upstream and should be fixed in later releases.
https://bugs.launchpad.net/desktopcouch/+bug/486797

rpmlint is throwing one bogus error message
python-couchdb.noarch: E: explicit-lib-dependency python-httplib2

This is a fault alarm.. python-httplib2 needs to be explicitly set as its the python bindins for the httplib2 library.


 -jef

Comment 6 Terje Røsten 2009-12-14 21:52:37 UTC
A should a documentation-only package be called -devel? 
Stranger things has happen.

I am still unsure about

Requires:       couchdb

Any new input here?

Well, well, this seems good. Will do a proper review soon.

 - Terje

Comment 7 Jef Spaleta 2009-12-14 22:13:39 UTC
The packaging guidelines on the wiki state that if the documentation is development related... like API documentation it should go in the -devel subpackage instead of -doc.  So its a judgement call but in this case the situation exactly fits the example api documentation case. 


-jef

Comment 8 Terje Røsten 2009-12-15 17:59:40 UTC
OK, this seems mostly fine 

ok: rpmlint
ok: naming
ok: license BSD, every file seems to include header, good
ok: language
ok: md5sum: 
446b8377cf2ddda94d5f2df29c4f705f  CouchDB-0.6.tar.gz
446b8377cf2ddda94d5f2df29c4f705f  CouchDB-0.6.tar.gz.1
ok: source url
ok: dirs
ok: perms
ok: utf-8

koji is very slow, not possible to do a test build, local build seems fine.

package the latest release: 0.6.1 and be more explicit in %files listing
(makes  things a lot easier for a new maintainer) and I will approve the
package.

Comment 9 Jef Spaleta 2009-12-15 19:00:58 UTC
0.6.1 doesn't need the shebang patch!

Is this good enough for the detail level in the files sections?

%files
%defattr(-,root,root,-)
%doc ChangeLog.txt COPYING README.txt
%{_bindir}/couchdb-dump
%{_bindir}/couchdb-load
%{_bindir}/couchdb-replicate
%{_bindir}/couchpy
%{python_sitelib}/CouchDB-%{version}-py2.5.egg-info
%{python_sitelib}/couchdb

%files devel
%defattr(-,root,root,-)
%doc doc/api doc/index.html

Comment 10 Terje Røsten 2009-12-15 19:17:53 UTC
Nice!

  Package is APPROVED.

Comment 11 Jef Spaleta 2009-12-15 19:51:38 UTC
New Package CVS Request
=======================
Package Name: python-couchdb
Short Description: A Python library for working with CouchDB
Owners: jspaleta sdz
Branches: F-12 F-13
InitialCC:

Comment 12 Sebastian Dziallas 2009-12-15 22:54:28 UTC
Cool, thanks everybody for taking on this one and sorry for being so calm lately, I've just been a bit swamped with work lately. Nice work!

Comment 13 Kevin Fenzi 2009-12-16 05:28:59 UTC
we don't have F-13 branches yet. Otherwise cvs done.

Comment 14 Jef Spaleta 2009-12-17 01:04:09 UTC
rawhide build complete... just needed to make one small buildrequires change to add python-setuptools.


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