Bug 1312408 - Review Request: python-sphinx-testing - Testing utility classes and functions for Sphinx extensions
Summary: Review Request: python-sphinx-testing - Testing utility classes and functions...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Zbigniew Jędrzejewski-Szmek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1312410
TreeView+ depends on / blocked
 
Reported: 2016-02-26 16:05 UTC by Jerry James
Modified: 2016-03-05 15:59 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-03-05 15:59:57 UTC
Type: ---
Embargoed:
zbyszek: fedora-review+


Attachments (Terms of Use)

Description Jerry James 2016-02-26 16:05:16 UTC
Spec URL: https://jjames.fedorapeople.org/python-sphinx-testing/python-sphinx-testing.spec
SRPM URL: https://jjames.fedorapeople.org/python-sphinx-testing/python-sphinx-testing-0.7.1-1.fc24.src.rpm
Fedora Account System Username: jjames
Description: Sphinx-testing provides testing utility classes and functions for Sphinx extensions.

Comment 1 Zbigniew Jędrzejewski-Szmek 2016-02-28 02:18:44 UTC
The description is unclear: is it classes for writing unit tests? Can you reword the description so it unambiguous what the purpose of the package is.

In check, use "nosetests-%{python3_version}", don't hardcode the version.
And nosetests-%{python2_version}" for the python 2 tests.

When you remove the shebang line, the file is modified, and there rule that timestamps of unmodified files should be preserved does not apply anymore. You can just use plain old "sed -i".

Would it work if you built python 2 and 3 versions from the same build directory?

Comment 2 Jerry James 2016-03-02 05:03:41 UTC
(In reply to Zbigniew Jędrzejewski-Szmek from comment #1)
> The description is unclear: is it classes for writing unit tests? Can you
> reword the description so it unambiguous what the purpose of the package is.

I have attempted to do so.

> In check, use "nosetests-%{python3_version}", don't hardcode the version.
> And nosetests-%{python2_version}" for the python 2 tests.

Ah, I didn't understand that that was the python version.  That would be a useful tip to put on the python packaging guidelines page, I think.

> When you remove the shebang line, the file is modified, and there rule that
> timestamps of unmodified files should be preserved does not apply anymore.
> You can just use plain old "sed -i".

Okay, done.

> Would it work if you built python 2 and 3 versions from the same build
> directory?

Maybe, but I don't want to.  I prefer to keep separate builds in separate directories so that there is no possibility of cross contamination.

New URLs:
Spec URL: https://jjames.fedorapeople.org/python-sphinx-testing/python-sphinx-testing.spec
SRPM URL: https://jjames.fedorapeople.org/python-sphinx-testing/python-sphinx-testing-0.7.1-2.fc24.src.rpm

Comment 3 Zbigniew Jędrzejewski-Szmek 2016-03-02 14:11:30 UTC
SRPM url gives 404.

Comment 4 Jerry James 2016-03-03 03:13:36 UTC
Oops, mock is producing fc25 instead of fc24 now.  Try this instead:

https://jjames.fedorapeople.org/python-sphinx-testing/python-sphinx-testing-0.7.1-2.fc25.src.rpm

Comment 5 Zbigniew Jędrzejewski-Szmek 2016-03-03 12:47:20 UTC
+ latest version
+ license is acceptable (BSD)
+ license file is present, %license is used
+ provides/requires look OK
+ python_provide is present
+ no scriptlets present or necessary
+ %check is present, tests pass

Package is APPROVED.

That said, let me return to the question of separate build dirs: people used to do that, because it made sense with 2to3. But fortunately we've mostly gotten rid of that, and "building" is just a step of copying a few files to build/. I just checked with 'diff -r /usr/share/doc/python2-sphinx-testing /usr/share/doc/python3-sphinx-testing' and '/usr/lib/python3.5/site-packages/sphinx_testing /usr/lib/python2.7/site-packages/sphinx_testing', and there's no difference apart from the .py[co] files. Upstream Python now supports using the same source for 2.7 and 3.x much better than it used to, and it also supports using the same dir for *binary* files thanks to the version specific extensions. Fedora guidelines don't recommend using separate build directories any more. So creating a separate build dir just makes everything more complicated for no good reason. If you *prefer* to do it this way, that's OK, but I still think it's unnecessary.

Comment 6 Zbigniew Jędrzejewski-Szmek 2016-03-03 12:48:08 UTC
FTR, rpmlint:
3 packages and 0 specfiles checked; 0 errors, 0 warnings.

Comment 7 Jerry James 2016-03-04 21:06:26 UTC
Thank you for the review.  Okay, I will experiment with not using separate build dirs and see how that works.  If I'm sufficiently reassured, I'll convert a few other packages I tend over to a single build dir, too.

Package creation requested.

Comment 8 Gwyn Ciesla 2016-03-04 22:24:46 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/python-sphinx-testing


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