Bug 1048045

Summary:	Review Request: python-sphinx-theme-better - A Better Sphinx Theme
Product:	[Fedora] Fedora	Reporter:	Sandro Mani <manisandro>
Component:	Package Review	Assignee:	Christopher Meng <i>
Status:	CLOSED RAWHIDE	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	i, package-review
Target Milestone:	---	Flags:	i: fedora-review+ gwync: fedora-cvs+
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2014-01-06 14:35:16 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 Sandro Mani 2014-01-03 01:40:18 UTC

Spec URL: http://smani.fedorapeople.org/review/python-sphinx-theme-better.spec
SRPM URL: http://smani.fedorapeople.org/review/python-sphinx-theme-better-0.1.5-1.fc21.src.rpm
Description: A Better Sphinx Theme
Fedora Account System Username: smani

Note: I included the with_python3 conditionals since python-pillow needs this package and python-pillow is in RHEL7 where python3 is not included.

Comment 1 Christopher Meng 2014-01-03 02:06:27 UTC

Is this one Fedora-only?

Comment 2 Sandro Mani 2014-01-03 08:08:27 UTC

I'll be doing fedora only. Actually, on second thoughts, I guess pillow won't get updated in RHEL7 at this time anyway (since it is already beta), so the conditionals can also be dropped.

Comment 3 Christopher Meng 2014-01-03 10:55:57 UTC

I intended to let you change the python2 macro from un-versioned to versioned like

%{__python}

to

%{__python2}.

You should add conditional lines for this.

Comment 4 Sandro Mani 2014-01-03 19:43:19 UTC

How about just doing
%global __python2 %{__python}
an possibly wrapping that in a conditional block when python 3 becomes default (F21 or F22 I guess)?

Comment 5 Christopher Meng 2014-01-05 09:44:22 UTC

Yes, add this line in the first block of %if 0%{?rhel} == 7, ALSO don't forget %{python_sitelib}.

Once new SRPM uploaded, I'll do a full review.

Comment 6 Sandro Mani 2014-01-05 17:45:11 UTC

Spec URL: http://smani.fedorapeople.org/review/python-sphinx-theme-better.spec
SRPM URL: http://smani.fedorapeople.org/review/python-sphinx-theme-better-0.1.5-2.fc21.src.rpm

%changelog
* Sun Jan 05 2014 Sandro Mani <manisandro> - 0.1.5-2
- Use %%{__python2} and %%{python2_sitelib}, define these macros for rhel7

Comment 7 Christopher Meng 2014-01-05 18:22:45 UTC

Ask upstream of patching these with license header:

Unknown or generated
--------------------
sphinx-better-theme-0.1.5/better/__init__.py
sphinx-better-theme-0.1.5/docs/conf.py
sphinx-better-theme-0.1.5/setup.py

And notify him of unusual 600 perms of the tarball.

--------------------

Hint: Instead of renaming the tarball everytime you download, why not directly download good-named one from pypi?

https://pypi.python.org/pypi/sphinx-better-theme

--------------------
PACKAGE APPROVED.

Comment 8 Sandro Mani 2014-01-05 18:38:38 UTC

Good point, I'll change the download link. Thanks for the review!

New Package SCM Request
=======================
Package Name: python-sphinx-theme-better
Short Description: A Better Sphinx Theme
Owners: smani
Branches:
InitialCC:

Comment 9 Gwyn Ciesla 2014-01-06 14:10:03 UTC

Git done (by process-git-requests).