Bug 1244014 - Review Request: python-ddt - Data-Driven/Decorated Tests
Summary: Review Request: python-ddt - Data-Driven/Decorated Tests
Keywords:
Status: CLOSED ERRATA
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: 1049250
TreeView+ depends on / blocked
 
Reported: 2015-07-16 21:28 UTC by Carl George 🤠
Modified: 2015-08-11 02:13 UTC (History)
2 users (show)

Fixed In Version: python-ddt-1.0.0-2.fc22
Clone Of:
Environment:
Last Closed: 2015-08-11 02:13:56 UTC
Type: ---
Embargoed:
zbyszek: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Carl George 🤠 2015-07-16 21:28:37 UTC
Spec URL: https://carlgeorge.fedorapeople.org/python-ddt/SPECS/python-ddt.spec
SRPM URL: https://carlgeorge.fedorapeople.org/python-ddt/SRPMS/python-ddt-1.0.0-1.fc22.src.rpm
Description: DDT (Data-Driven Tests) allows you to multiply one test case by running it with different test data, and make it appear as multiple test cases.
Fedora Account System Username: carlgeorge

I need this package in order to update python-falcon (bug 1049250).

Comment 1 Carl George 🤠 2015-07-16 21:36:11 UTC
[carl@george python-ddt]$ rpmlint \
> SPECS/python-ddt.spec \
> SRPMS/python-ddt-1.0.0-1.fc22.src.rpm \
> RPMS/python-ddt-1.0.0-1.fc22.noarch.rpm \
> RPMS/python3-ddt-1.0.0-1.fc22.noarch.rpm 
3 packages and 1 specfiles checked; 0 errors, 0 warnings.

Comment 2 Zbigniew Jędrzejewski-Szmek 2015-07-18 05:36:50 UTC
So, the packaging is OK, but it looks like it can be simplified a lot. Have you tried running %build and %install without creating  a separate py3k directory? This package doesn't use 2to3 so this should work juts as well.

Provides: python2-%{srcname} is nonstandard, I have never seen any python 2 packages provide python2-* name.

You can use %global _docdir_fmt %{name} to use a common license and documentation directories.

Summary or %description should say what this package is in more basic term (e.g. a library which provides a testing framework).

Comment 3 Zbigniew Jędrzejewski-Szmek 2015-07-18 06:02:06 UTC
(In reply to Zbigniew Jędrzejewski-Szmek from comment #2)
> Provides: python2-%{srcname} is nonstandard, I have never seen any python 2
> packages provide python2-* name.
Oh, it's new in the guidelines. Please ignore my comment.

Comment 4 Carl George 🤠 2015-07-20 18:57:05 UTC
> Have you tried running %build and %install without creating  a separate py3k directory? This package doesn't use 2to3 so this should work juts as well.

I have not tried it that way.  I just did it the same way I saw several other module packages do it (pip, setuptools, six, click).  The only documentation I could find about this says the macro should be used when the same source is used for both python2 and python3 [1], which is the case here.

> You can use %global _docdir_fmt %{name} to use a common license and documentation directories.

I'm not following what the point of this would be.  I thought the current project goal was to separate license files from documentation [2]?

> Summary or %description should say what this package is in more basic term (e.g. a library which provides a testing framework).

I thought that was clear in the summary, but I am not a wordsmith, so I'm open to more detailed suggestions here.  I just copied the current summary and description from the setup.py and README.md files.  Maybe the long_description in the setup.py would be more appropriate for the RPM description?

> long_description='A library to multiply test cases',


[1]: https://fedoraproject.org/wiki/Python3.4GuidlinesDraft#Macros
[2]: https://fedoraproject.org/wiki/Changes/Use_license_macro_in_RPMs_for_packages_in_Cloud_Image#Summary

Comment 5 Zbigniew Jędrzejewski-Szmek 2015-07-20 19:51:42 UTC
(In reply to Carl George from comment #4)
> > Have you tried running %build and %install without creating  a separate py3k directory? This package doesn't use 2to3 so this should work juts as well.
> 
> I have not tried it that way.  I just did it the same way I saw several
> other module packages do it (pip, setuptools, six, click).  The only
> documentation I could find about this says the macro should be used when the
> same source is used for both python2 and python3 [1], which is the case here.
This idea of using py3dir originated when sources were "translated" when building
the package, using 2to3. After this step was done, sources were incompatible with
Python 2 and couldn't be used any more, so there was no choice except to use
a second source directory. Another related reason could have been that when
building for multiple python versions, .so, .pyc, and .pyo files would be overwritten.
Starting with python3 the version of the interpreter is included in the file name
(.cpython-34m.so, .cpython-34.pyc), so build products do not conflict. Python 2.x
does not use this, but because we are building for only one version of Python 2
and one version of Python 3, the conflict is avoided anyway.

Using 2to3 was cumbersome, and fortunately almost no projects do that any more.
The guidelines do not require a separate directory to be used, and only prescribe
the name of the directory to use if it used. Most new packages don't use a separate
directory. If you insist on using two source directories, I won't press the point,
but please note that
a) this is totally obsolete because the reasons for it are gone
b) it clutters the spec file and the file system

> > You can use %global _docdir_fmt %{name} to use a common license and documentation directories.
> 
> I'm not following what the point of this would be.  I thought the current
> project goal was to separate license files from documentation [2]?
It's about sharing the %doc dir between subpackages, and the %license dir
between subpackages. You'd have:

/usr/share/doc/python-ddt/README
/usr/share/licenses/python-ddt/LICENSE

and those two files would be owned by both python-ddt and python3-ddt.
The motivation is that when a user looks into /usr/share/doc/, if
both packages are installed, he or she has to inspect both READMEs to
realize that they are identical, so it's just simpler to provide just
one file.

> > Summary or %description should say what this package is in more basic term (e.g. a library which provides a testing framework).
> 
> I thought that was clear in the summary, but I am not a wordsmith, so I'm
> open to more detailed suggestions here.  I just copied the current summary
> and description from the setup.py and README.md files.  Maybe the
> long_description in the setup.py would be more appropriate for the RPM
> description?
> 
> > long_description='A library to multiply test cases',
Actually, what is missing, is the information that this is not a testing
framework, but that it is to be used with unittests or nose. So maybe
something like this:

%description
DDT (Data-Driven Tests) allows you to multiply one test case by running
it with different test data, and make it appear as multiple test cases.
It is used in combination with other testing frameworks like unittest
and nose.


Hm, and what about documentation? There are some sphinx docs which could
be built and distributed.

Comment 6 Carl George 🤠 2015-07-20 20:35:58 UTC
Thanks for the clarification on those points.  I'm updating the spec file now to remove the separate build directory, combine the doc/license directories between PY2/PY3 packages, and updated the description.

Regarding the sphinx documentation, it looks like that would require sphinxcontrib-programoutput, which isn't currently packaged in Fedora that I'm aware of.

Comment 7 Carl George 🤠 2015-07-20 21:02:11 UTC
Recommended changes have been implemented, please proceed with the review.

Spec URL: https://carlgeorge.fedorapeople.org/python-ddt/SPECS/python-ddt.spec
SRPM URL: https://carlgeorge.fedorapeople.org/python-ddt/SRPMS/python-ddt-1.0.0-2.fc22.src.rpm

Comment 8 Zbigniew Jędrzejewski-Szmek 2015-07-20 23:44:14 UTC
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated

===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated". 9 files have unknown license. Detailed
     output of licensecheck in /var/tmp/1244014-python-ddt/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 20480 bytes in 2 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

Python:
[x]: Python eggs must not download any dependencies during the build
     process.
[x]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Binary eggs must be removed in %prep

Rpmlint
-------
python-ddt.noarch: W: spelling-error %description -l en_US unittest -> unit test, unit-test, unities
python3-ddt.noarch: W: spelling-error %description -l en_US unittest -> unit test, unit-test, unities
python-ddt.src: W: spelling-error %description -l en_US unittest -> unit test, unit-test, unities

All false positives.

Requires
--------
python-ddt (rpmlib, GLIBC filtered):
    python(abi)

python3-ddt (rpmlib, GLIBC filtered):
    python(abi)

Provides
--------
python-ddt:
    python-ddt
    python2-ddt

python3-ddt:
    python3-ddt

Package is APPROVED.

Comment 9 Carl George 🤠 2015-07-21 13:57:29 UTC
New Package SCM Request
=======================
Package Name: python-ddt
Short Description: A Python library to multiply test cases
Upstream URL: https://github.com/txels/ddt
Owners: carlgeorge
Branches: f22 f23
InitialCC:

Comment 10 Gwyn Ciesla 2015-07-23 13:46:17 UTC
Git done (by process-git-requests).

Comment 11 Fedora Update System 2015-07-27 20:47:12 UTC
python-ddt-1.0.0-2.fc22 has been submitted as an update for Fedora 22.
https://admin.fedoraproject.org/updates/python-ddt-1.0.0-2.fc22

Comment 12 Fedora Update System 2015-07-30 00:54:12 UTC
python-ddt-1.0.0-2.fc22 has been pushed to the Fedora 22 testing repository.

Comment 13 Fedora Update System 2015-08-11 02:13:56 UTC
python-ddt-1.0.0-2.fc22 has been pushed to the Fedora 22 stable repository.


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