Bug 1312408 - Review Request: python-sphinx-testing - Testing utility classes and functions for Sphinx extensions
Review Request: python-sphinx-testing - Testing utility classes and functions...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Zbigniew Jędrzejewski-Szmek
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 1312410
  Show dependency treegraph
 
Reported: 2016-02-26 11:05 EST by Jerry James
Modified: 2016-03-05 10:59 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2016-03-05 10:59:57 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
zbyszek: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Jerry James 2016-02-26 11:05:16 EST
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-27 21:18:44 EST
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 00:03:41 EST
(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 09:11:30 EST
SRPM url gives 404.
Comment 4 Jerry James 2016-03-02 22:13:36 EST
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 07:47:20 EST
+ 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 07:48:08 EST
FTR, rpmlint:
3 packages and 0 specfiles checked; 0 errors, 0 warnings.
Comment 7 Jerry James 2016-03-04 16:06:26 EST
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 17:24:46 EST
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.