I am taking this.

1st of all, do you have any comments on having the package to own 
/usr/lib/pythonX/site-packages/tests 
directory and having files in that directory?

I mean, what if there was a package named python-tests? Also, what if another package goes on with the same decision and wants to own, let's say 
/usr/lib/python3.5/site-packages/tests/util.py
?

Hi,

You right directory /usr/lib/pythonX/site-packages/tests isn't necessary for package. 

I delete this directory after test rpm.

Best regard

Hi David,

Naming:

- All the Flask related packages are named with lower case. Did you consider that when naming this package?

License:

- The license file you are using belongs to the "Flask Sphinx Style" used in the documentation, not to this package.

- Note that since this package is shipped under a BSD license, it must contain the full license text. The text is present in both files shipped in the package (__init__.py and wrappers.py), but it would be really nice if you considered asking upstream to also include the text in a separate file (like LICENSE or COPYING). This is not a blocker for this review, since the text is already available in the files I mentioned.

Tests:

- Upstream provides a test suite. Is there any reason for not running the tests in %check?

Documentation:

- There are some import errors being raised while building documentation, did you check that?

Hi Athos,

I fix naming and add tests :

Spec URL: https://hvad.fedorapeople.org/fedora/python-Flask-PyMongo/python-flask-pymongo.spec

SRPM URL: https://hvad.fedorapeople.org/fedora/python-Flask-PyMongo/python-flask-pymongo-0.5.1-1.fc25.src.rpm

I'll contact upstream. Could it pass anyway ? 

Best regard

Hi David,


- Upstream already ships a LICENSE file [1], you should use it (no need to contact upstream).

- For the python2- subpackage, you want to use the python2-* Requires and BR, not just python-* (they are already available for your dependencies).

- The licence file you are shipping in python2 and python3 subpackages belongs to the documentation theme: it should go in the documentation subpackage instead, not in the binary packages.

- You are removing the tests directory in %build, so there's no tests to run in %check.

- There are (still) some import errors being raised while building documentation, did you check that [2]?

[1] https://github.com/dcrosta/flask-pymongo/blob/master/LICENSE

[2]

Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.tEhIGH
...
+ sphinx-build docs html

...
looking for now-outdated files... /builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:59: WARNING: autodoc: failed to import method u'Collection.find_one_or_404' from module u'flask_pymongo.wrappers'; the following exception was raised:
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object
    __import__(self.modname)
  File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module>
    from bson.errors import InvalidId
ImportError: No module named bson.errors
/builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:61: WARNING: autodoc: failed to import method u'PyMongo.send_file' from module u'flask_pymongo'; the following exception was raised:
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object
    __import__(self.modname)
  File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module>
    from bson.errors import InvalidId
ImportError: No module named bson.errors
/builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:63: WARNING: autodoc: failed to import method u'PyMongo.save_file' from module u'flask_pymongo'; the following exception was raised:
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object
    __import__(self.modname)
  File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module>
    from bson.errors import InvalidId
ImportError: No module named bson.errors
/builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:65: WARNING: autodoc: failed to import class u'BSONObjectIdConverter' from module u'flask_pymongo'; the following exception was raised:
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object
    __import__(self.modname)
  File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module>
    from bson.errors import InvalidId
ImportError: No module named bson.errors
/builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:193: WARNING: autodoc: failed to import data u'ASCENDING' from module u'flask_pymongo'; the following exception was raised:
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object
    __import__(self.modname)
  File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module>
    from bson.errors import InvalidId
ImportError: No module named bson.errors
/builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:195: WARNING: autodoc: failed to import data u'DESCENDING' from module u'flask_pymongo'; the following exception was raised:
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object
    __import__(self.modname)
  File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module>
    from bson.errors import InvalidId
ImportError: No module named bson.errors
/builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:201: WARNING: autodoc: failed to import class u'PyMongo' from module u'flask_pymongo'; the following exception was raised:
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object
    __import__(self.modname)
  File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module>
    from bson.errors import InvalidId
ImportError: No module named bson.errors
/builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:204: WARNING: autodoc: failed to import class u'Collection' from module u'flask_pymongo.wrappers'; the following exception was raised:
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object
    __import__(self.modname)
  File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module>
    from bson.errors import InvalidId
ImportError: No module named bson.errors
/builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:217: WARNING: autodoc: failed to import class u'MongoClient' from module u'flask_pymongo.wrappers'; the following exception was raised:
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object
    __import__(self.modname)
  File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module>
    from bson.errors import InvalidId
ImportError: No module named bson.errors
/builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:220: WARNING: autodoc: failed to import class u'MongoReplicaSetClient' from module u'flask_pymongo.wrappers'; the following exception was raised:
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object
    __import__(self.modname)
  File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module>
    from bson.errors import InvalidId
ImportError: No module named bson.errors
/builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:223: WARNING: autodoc: failed to import class u'Database' from module u'flask_pymongo.wrappers'; the following exception was raised:
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object
    __import__(self.modname)
  File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module>
    from bson.errors import InvalidId
ImportError: No module named bson.errors
none found

 - Use BuildRequires:  python3-sphinx, not python-sphinx

 - Download the LICENSE file and install it:

Source1:        https://raw.githubusercontent.com/dcrosta/flask-pymongo/master/LICENSE

   And:

%prep
%autosetup -n %{pypi_name}-%{version}
rm -rf %{pypi_name}.egg-info

cp %{S:1} .

   Then install it in %files:

%files -n python2-flask-pymongo
%license LICENSE
%doc README.md
%{python2_sitelib}/flask_pymongo
%{python2_sitelib}/Flask_PyMongo-%{version}-py*egg-info

%files -n python3-flask-pymongo
%license LICENSE
%doc README.md
%{python3_sitelib}/flask_pymongo
%{python3_sitelib}/Flask_PyMongo-%{version}-py*egg-info

%files -n python-flask-pymongo-doc
%license docs/_themes/LICENSE
%doc html 

 - Bump to 0.5.2

 - Generate the docs with:

# generate html docs 
PYTHONPATH=${PWD} sphinx-build-3 docs html
# remove the sphinx-build leftovers
rm -rf html/.{doctrees,buildinfo}

 - Add the required BR to generate the docs:

BuildRequires:  python2dist(flask) >= 0.8
BuildRequires:  python2dist(pymongo) >= 2.5
BuildRequires:  python2-pymongo-gridfs

BuildRequires:  python3dist(flask) >= 0.8
BuildRequires:  python3dist(pymongo) >= 2.5
BuildRequires:  python3-pymongo-gridfs

 - Not useful:

rm -rf %{python2_sitelib}/tests

 - The tests are not included with the Pypi anchive, remove:

%check
%{__python2} setup.py test
%{__python3} setup.py test

 - You could use the new python2dist/python3dist macros:

BuildRequires:  python2-devel
BuildRequires:  python2dist(setuptools)
BuildRequires:  python2dist(flask) >= 0.8
BuildRequires:  python2dist(pymongo) >= 2.5
BuildRequires:  python2-pymongo-gridfs
 
BuildRequires:  python3-devel
BuildRequires:  python3dist(setuptools)
BuildRequires:  python3dist(sphinx)
BuildRequires:  python3dist(flask) >= 0.8
BuildRequires:  python3dist(pymongo) >= 2.5
BuildRequires:  python3-pymongo-gridfs


   And:

%package -n     python2-flask-pymongo
Summary:        %{summary}
%{?python_provide:%python_provide python2-flask-pymongo}
 
Requires:       python2dist(flask) >= 0.8
Requires:       python2dist(pymongo) >= 2.5

   And:

%package -n     python3-flask-pymongo
Summary:        %{summary}
%{?python_provide:%python_provide python3-flask-pymongo}
 
Requires:       python3dist(flask) >= 0.8
Requires:       python3dist(pymongo) >= 2.5

This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time, but it seems
that the review is still being working out by you. If this is right, please
respond to this comment clearing the NEEDINFO flag and try to reach out the
submitter to proceed with the review.

If you're not interested in reviewing this ticket anymore, please clear the
fedora-review flag and reset the assignee, so that a new reviewer can take
this ticket.

Without any reply, this request will shortly be resetted.

Hi David,

Are you still interested in proceeding with this one?