Bug 1244014
| Summary: | Review Request: python-ddt - Data-Driven/Decorated Tests | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Carl George 🤠<carlwgeorge> |
| Component: | Package Review | Assignee: | Zbigniew Jędrzejewski-Szmek <zbyszek> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | package-review, zbyszek |
| Target Milestone: | --- | Flags: | zbyszek:
fedora-review+
gwync: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | python-ddt-1.0.0-2.fc22 | Doc Type: | Bug Fix |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2015-08-11 02:13:56 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Bug Depends On: | |||
| Bug Blocks: | 1049250 | ||
|
Description
Carl George ðŸ¤
2015-07-16 21:28:37 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.
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).
(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. > 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 (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. 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. 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 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.
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: Git done (by process-git-requests). 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 python-ddt-1.0.0-2.fc22 has been pushed to the Fedora 22 testing repository. python-ddt-1.0.0-2.fc22 has been pushed to the Fedora 22 stable repository. |