Bug 754092

Summary: Review Request: python-restauth - Python RestAuth reference implementation
Product: [Fedora] Fedora Reporter: Jan Kaluža <jkaluza>
Component: Package ReviewAssignee: Jaroslav Škarvada <jskarvad>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: jskarvad, mati, notting, package-review
Target Milestone: ---Flags: jskarvad: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: python-restauth-0.5.1-3.fc16 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-12-13 21:51:58 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Jan Kaluža 2011-11-15 11:51:24 UTC
Spec URL: http://jkaluza.fedorapeople.org/python-restauth.spec
SRPM URL: http://jkaluza.fedorapeople.org/python-restauth-0.5.1-1.fc15.src.rpm
Description:

Hi,
I've finished packaging of python-mimeparse Python module. This module implements all features of the RestAuth specification and is a reference implementation. It is written in pure python. It generally works with python 2.6 or later (including python3).

It depends on python-restauth-common: https://bugzilla.redhat.com/show_bug.cgi?id=754088
Consider reviewing also python-restauth-common before reviewing this package please.

$ rpmlint python-restauth-0.5.1-1.fc15.src.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

$ rpmlint python-restauth-0.5.1-1.fc15.noarch.rpm python3-restauth-0.5.1-1.fc15.noarch.rpm 
2 packages and 0 specfiles checked; 0 errors, 0 warnings.

Comment 1 Jaroslav Škarvada 2011-11-21 13:08:29 UTC
MUST items:
[YES] rpmplint is silent
[YES] Package meets naming guidelines.
[YES] Package meets packaging guidelines
  I am not sure about the API/ABI compatibility policy of this project.
  Shouldn't be there version requirement for python-restauth-common?
[YES] Spec file matches base package name.
[YES] License file is present, matching with spec file.
[YES] Licensing Guidelines are met.
[!] Spec file is legible and in American English.
  I would prefer summary like: "Reference implementation of RestAuth specification in Python"
  or similar

  I would tune the description a bit. I wouldn't note the python versions there and I would probably re-word the text. Maybe you could also very briefly describe what the RestAuth is (e.g. "The RestAuth project is a system providing shared authentication, authorization and preferences.").
[YES] Sources match upstream.
[YES] Package builds OK.
[!] BuildRequires are correct.
  I cannot find the python-setuptools-devel in rawhide.
  Is the python-setuptools really needed? It seems to build OK without it.

[YES] Package doesn't bundle copies of system libraries.
[YES] Package owns all the directories it creates.
[YES] Package has no duplicity in %files.
[YES] Permission on files are set properly.
[NO] Spec file has consistant macro usage.
  Please use %{optflags} instead of $RPM_OPT_FLAGS or $RPM_BUILD_ROOT instead of %{buildroot}.
[YES] Package is code or permissible content.
[YES] %doc files don't affect runtime.
[YES] Package doesn't own files/directories that other packages own.
[YES] Spec file is valid UTF-8.

Should items:
[YES] Package builds in mock.
[YES] Package uses sane scriptlets.

Some more comments:
There is extra space in the second %doc (only cosmetic issue :)

%{!?python_sitelib: %global python_sitelib %(%{__python} -c "from distutils.sysconfig import get_python_lib; print get_python_lib()")}
is probably not needed any more.

The defattr is also not needed.

AFAIK the above have only sense if it is planned to package for RHEL-5 EPEL. If so there should be also more additions (e.g. %clean section, ...).

Please consider running the included test-suite as a part of the build process.

Please consider packaging the docs.

Please consider packaging the example script.

Comment 2 Mathias Ertl 2011-11-21 13:36:15 UTC
Version 0.5.1 of python-restauth depends on python-restauth-common >= 0.5.1. (The versions are not by definition the same, they just happen to coincide).

greetings, Mati

Comment 3 Jan Kaluža 2011-11-30 07:17:57 UTC
Fixed specfile and srpm:

Spec URL: http://jkaluza.fedorapeople.org/python-restauth.spec
SRPM URL: http://jkaluza.fedorapeople.org/python-restauth-0.5.1-2.fc16.src.rpm

I think all problems should be fixed. test-suite is not run, because it's currently broken. Upstream has been informed and I will use it when it will work again.

Comment 4 Jaroslav Škarvada 2011-11-30 13:13:43 UTC
Is the BuildRequires:  python3-setuptools needed? It seems to build OK without it. Also consider packaging demo script from examples (its shebang would probably require tweaking).

Comment 5 Jan Kaluža 2011-12-01 09:10:59 UTC
Fixed specfile and srpm:

Spec URL: http://jkaluza.fedorapeople.org/python-restauth.spec
SRPM URL: http://jkaluza.fedorapeople.org/python-restauth-0.5.1-3.fc16.src.rpm

Examples should be included and setuptools removed.

Comment 6 Jaroslav Škarvada 2011-12-01 12:50:40 UTC
Looks OK to me, giving fedora_review +. You could use the sed -i, to simplify the code for shebang removal.

Comment 7 Jan Kaluža 2011-12-02 16:54:08 UTC
New Package SCM Request
=======================
Package Name: python-restauth
Short Description: reference implementation of RestAuth specification in Python
Owners: jkaluza
Branches: f15 f16
InitialCC:

Comment 8 Gwyn Ciesla 2011-12-02 16:55:11 UTC
Git done (by process-git-requests).

Comment 9 Fedora Update System 2011-12-05 09:48:22 UTC
python-restauth-0.5.1-3.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/python-restauth-0.5.1-3.fc16

Comment 10 Fedora Update System 2011-12-06 01:04:31 UTC
python-restauth-0.5.1-3.fc16 has been pushed to the Fedora 16 testing repository.

Comment 11 Fedora Update System 2011-12-13 21:51:58 UTC
python-restauth-0.5.1-3.fc16 has been pushed to the Fedora 16 stable repository.