Bug 1272187

Some initial comments prior to a formal review:

 * Current upstream version is 1.4.2, not 1.4.1

 * I'd suggest replicating the runtime Requires: as BuildRequires:
   That way, not only should any future test suite run OK, but you'll
   find out at build-time rather than install-time if there are any
   dependency issues.

 * Version requirements from setup.py are not reflected in rpm deps.

 * Dependency on python-six is missing.

 * Comment referring to BZ 1205170 can probably be dropped.

 * Shellbang removal should be done in %prep rather than %install; if
   done before the copy to %{py3dir}, it can be done just once.

 * Typo in comment for 1.4.0-1: pythn3 missing "o".

Thanks Paul,

could it be that you took a look at some older packages? The ones in the 
description had most of the comments fixed.

(In reply to Paul Howarth from comment #1)
> Some initial comments prior to a formal review:
> 
>  * Current upstream version is 1.4.2, not 1.4.1

Done (see above)

>  * I'd suggest replicating the runtime Requires: as BuildRequires:
>    That way, not only should any future test suite run OK, but you'll
>    find out at build-time rather than install-time if there are any
>    dependency issues.

Ack, agreed

>  * Version requirements from setup.py are not reflected in rpm deps.

Done (see above)
 
>  * Dependency on python-six is missing.

Done (see above)

>  * Comment referring to BZ 1205170 can probably be dropped.

Done (see above)

>  * Shellbang removal should be done in %prep rather than %install; if
>    done before the copy to %{py3dir}, it can be done just once.

Nice catch, fixed
 
>  * Typo in comment for 1.4.0-1: pythn3 missing "o".

Done (see above)

Thanks for the feedback, Paul!

New packages:
Spec URL: http://acksyn.org/files/rpms/google-api-python-client/google-api-python-client.spec
SRPM URL: http://acksyn.org/files/rpms/google-api-python-client/google-api-python-client-1.4.2-2.fc23.src.rpm

I'm pretty sure I clicked on the link from this review ticket. Anyway, it's up to date now. I just looked at the python packaging guidelines (https://fedoraproject.org/wiki/Packaging:Python) and it looks like the python package boilerplate has changed since I last looked at a python package, with a view to making Python 3 the default python somewhere down the line. Given that I don't think you're targeting anything older than F-23 with this package, I'd suggest using the modern packaging style:

 * Use python2-* dependencies rather than python-* dependencies where possible (currently python-httplib2, python-oauth2client and python-uri-templates don't have the necessary provides for this as yet, which should get fixed really)

 * Rename the python-google-api-client package to python2-google-api-client

 * Use the %python_provide macro as per the guidelines and sample spec

 * Use %py2_build, %py3_build, %py2_install and %py3_install as per the sample spec

Again, if you're only targeting F-23 onwards, you can dispense with the python3 build conditional and just do it unconditionally, which would simplify the spec quite a bit. Or do you have thoughts of supporting EPEL-7 perhaps?

Thanks Paul!

I have no plans for now for EPEL-7 so I tweaked everything to make it more
conform to the latest python policy. I have just fixed up python-oauth2client and python-uri-templates, so it will need a bit of time until those python2-* 
deps show up in rawhide.

New packages:
Spec URL: http://acksyn.org/files/rpms/google-api-python-client/google-api-python-client.spec
SRPM URL: http://acksyn.org/files/rpms/google-api-python-client/google-api-python-client-1.4.2-3.fc23.src.rpm

Review
======

Provides
--------
$ rpm -qp --provides python2-google-api-client-1.4.2-3.fc24.noarch.rpm 
python-google-api-client = 1.4.2-3.fc24
python-google-api-client(x86-64) = 1.4.2-3.fc24
python2-google-api-client = 1.4.2-3.fc24

$ rpm -qp --provides python3-google-api-client-1.4.2-3.fc24.noarch.rpm 
python3-google-api-client = 1.4.2-3.fc24

Requires
--------
$ rpm -qp --requires python2-google-api-client-1.4.2-3.fc24.noarch.rpm 
python(abi) = 2.7
python-httplib2 >= 0.8
python2-oauth2client >= 1.4.6
python2-six >= 1.6.1
python2-uri-templates >= 0.6
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PartialHardlinkSets) <= 4.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(PayloadIsXz) <= 5.2-1

$ rpm -qp --requires python3-google-api-client-1.4.2-3.fc24.noarch.rpm 
python(abi) = 3.5
python3-httplib2 >= 0.8
python3-oauth2client >= 1.4.6
python3-six >= 1.6.1
python3-uri-templates >= 0.6
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(PayloadIsXz) <= 5.2-1

Review Checklist
----------------

- rpmlint is silent
- package and spec file naming OK; difference between source and binary
  package names may confuse some users but makes sense
- package meets guidelines, specifically the Python ones
- license is Apache V2, OK for Fedora, matches spec
- license text is packaged in %license
- spec file written in English and is legible
- source tarball matches upstream, including timestamp
- source builds OK for x86_64 in Rawhide
- buildreqs sane and (mostly) versioned as per upstream's setup.py
- no locale data to worry about
- no binary libraries to worry about
- no bundled libraries included
- package is not intended to be relocatable (good)
- directory ownership is correct
- no duplicate files
- permissions are sane
- macro usage is consistent
- code, not content
- no large documentation items
- docs don't affect runtime
- not a GUI app, no desktop file needed
- filenames are all ASCII
- no scriptlets present
- no inter-package dependency issues
- %python_provide macro used correctly
- upstream egg is removed in %prep
- no dependencies need to be downloaded for build or test
- built egg-info is shipped correctly

Nits
----
Could specify python version build requirements:
python2-devel >= 2.7 (or 2.6 if python-argparse added)
python3-devel >= 3.3

Could append "/" to directory names in %files list for clarity

Could comment about the code in %prep removing shellbangs without
losing timestamps

No blocker issues - package is APPROVED.

Thanks, Paul.

I requested the new package via pkgdb2. I shall try and address the nits you
pointed out before I import it.

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/google-api-python-client