Bug 1148529 - Review Request: python-flask-rstpages - Adds support for reStructuredText to a Flask application
Summary: Review Request: python-flask-rstpages - Adds support for reStructuredText to ...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Marek Bryša
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-10-01 16:07 UTC by Richard Marko
Modified: 2015-09-14 00:28 UTC (History)
6 users (show)

Fixed In Version: python-flask-rstpages-0.3-3.fc20
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-12-04 06:25:10 UTC
Type: ---
Embargoed:
mbrysa: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Richard Marko 2014-10-01 16:07:06 UTC
Spec URL: https://rmarko.fedorapeople.org/packages/python-flask-rstpages.spec
SRPM URL: https://rmarko.fedorapeople.org/packages/python-flask-rstpages-0.3-1.fc20.src.rpm
Description: Adds support for reStructuredText to a Flask application
Fedora Account System Username: rmarko

Comment 1 Dmitry Tantsur 2014-10-09 12:41:18 UTC
Hi! Some comments to this package:

1. Inconsistent python2 and python3 packages naming (note case):
python-flask-rstpages-0.3-1.fc20.noarch.rpm
python3-Flask-RSTPages-0.3-1.fc20.noarch.rpm
2. sphinx-build-3.3 docs html
I suggest using just sphinx-build-3 (it's also present). There's no point in assuming specific version of Python 3.
E.g. in F21 your package fails to build: http://koji.fedoraproject.org/koji/taskinfo?taskID=7811395
3. You should BuildRequire python[3]-flask, otherwise Sphinx fails with ImportError on building docs.
4. You have this line (likely due to following Python packaging guide):
find %{py3dir} -name '*.py' | xargs sed -i '1s|^#!python|#!%{__python3}|'
but the guide also states symmetric:
find -name '*.py' | xargs sed -i '1s|^#!python|#!%{__python2}|'
after Python 3 part.
5. %doc html docs/_themes/LICENSE LICENSE
Here one LICENSE file is overwriting another, so only the last is actually included.

Smaller things:

1. Somewhere your spec is using
 %if %{?with_python3}
somewhere
 %if 0%{?with_python3}
I guess it should be just
 %if 0%{?with_python3}
everywhere, but at least it should be consistent.
2. Just a suggestion: won't it be good to
Provide: Flask-RSTPages
so that people can easily find it?

F20 build for reference: http://koji.fedoraproject.org/koji/taskinfo?taskID=7811450

Comment 2 hguemar 2014-10-14 08:33:01 UTC
ACK, thanks Dmitry, this was a good review.
I would consider the "0%{?with_python3}" as mandatory as i

@Richard: please do the changes requested.
The Provide: Flask-RSTPages is not mandatory, but that's a good idea

Comment 4 Richard Marko 2014-10-30 12:53:20 UTC
Ping.

Comment 5 Marek Bryša 2014-11-21 13:10:38 UTC
Apparently, all the issues pointed out have been fixed.

Comment 6 Richard Marko 2014-11-21 15:51:51 UTC
New Package SCM Request
=======================
Package Name: python-flask-rstpages
Short Description: Adds support for reStructuredText to a Flask application
Upstream URL: https://github.com/saimn/flask-rstpages/
Owners: rmarko
Branches: f20 f21 el6 epel7
InitialCC: rmarko

Comment 7 Gwyn Ciesla 2014-11-21 18:01:10 UTC
Git done (by process-git-requests).

Comment 8 Fedora Update System 2014-11-24 14:03:10 UTC
python-flask-rstpages-0.3-3.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/python-flask-rstpages-0.3-3.fc20

Comment 9 Fedora Update System 2014-11-25 15:27:45 UTC
python-flask-rstpages-0.3-3.fc20 has been pushed to the Fedora 20 testing repository.

Comment 10 Fedora Update System 2014-12-04 06:25:10 UTC
python-flask-rstpages-0.3-3.fc20 has been pushed to the Fedora 20 stable repository.


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