Spec URL: https://rathann.fedorapeople.org/review/python-asynctest/python-asynctest.spec SRPM URL: https://rathann.fedorapeople.org/review/python-asynctest/python-asynctest-0.12.2-1.fc30.src.rpm Description: The package asynctest is built on top of the standard unittest module and cuts down boilerplate code when testing libraries for asyncio. Currently, asynctest targets the “selector” model, hence, some features will not (yet?) work with Windows’ proactor. Fedora Account System Username: rathann
Hello Dominik, I'm taking this review. I tried to run fedora-review for your spec file and it failed because of the test suite. It appears that the issue is already reported upstream [0]. We can proceed with the review by disabling the test suite for now (s/bcond_without check/bcond_with check/) if you like, or we can wait for upstream to fix the test suite. In the meantime, there are several other things that should be fixed in the spec file. * The summary and description lines should be no longer than 80 characters [1]. * Replace fancy single and double quotes (’ and ”) in description with standard ascii quotes (' and "). * Use the %pypi_source macro to define Source0 [2]. * Upstream uses setuptools in setup.py [3], so it needs to be listed as a build requirement. Furthermore, upstream specifically calls out setuptools >= 30.3.0 [4], so that minimum version requirement should be used with the build requirement. * The %python_provides macro should have an argument of python3-asynctest, not python3-mmtf. * Capitalize your changelog entry. * Instead of using the working directory for the tests, use the buildroot's sitelib by setting PYTHONPATH to %{buildroot}%{python3_sitelib}. [0]: https://github.com/Martiusweb/asynctest/issues/112 [1]: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_summary_and_description [2]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_source_files_from_pypi [3]: https://github.com/Martiusweb/asynctest/blob/v0.12.2/setup.py#L35 [4]: https://github.com/Martiusweb/asynctest/blob/v0.12.2/setup.py#L5
One more small thing, your multiline desc macro is adding an empty line at the beginning of the description. Description : The package asynctest is built on top of the standard unittest module and cuts down boilerplate code when testing libraries for asyncio. Currently, asynctest targets the “selector” model, hence, some features will not (yet?) work with Windows’ proactor. You can avoid that by moving %{desc} onto the same line as the %description's. -%description -%{desc} +%description %{desc} -%description -n python3-%{pname} -%{desc} +%description -n python3-%{pname} %{desc}
Thanks, Carl. I opted for marking the failing test as expected failure instead. Interestingly enough, it doesn't fail when run under plain rpmbuild or manually. It only fails when run as part of mock build. Spec URL: https://rathann.fedorapeople.org/review/python-asynctest/python-asynctest.spec SRPM URL: https://rathann.fedorapeople.org/review/python-asynctest/python-asynctest-0.12.2-2.fc30.src.rpm * Fri Jan 11 2019 Dominik Mierzejewski <dominik> 0.12.2-2 - ignore test failing under mock - remove extra newline from description
Please address the other items I listed in my first comment. I also noticed you removed the python_provide line entirely. It's still needed, and should look like this for your spec file: %{?python_provide:%python_provide python3-asynctest} See https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_provides for more details.
(In reply to Carl George from comment #4) > Please address the other items I listed in my first comment. > > I also noticed you removed the python_provide line entirely. It's still > needed, and should look like this for your spec file: > > %{?python_provide:%python_provide python3-asynctest} > > See > https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_provides > for more details. My reading of the above is that it's not necessary if subpackage is named python3-%{pypi_name}. It seems to evaluate to empty string, too. [mockbuild@06036e176d6b4146ad180aa2c6c53351 ~]$ rpm --eval '%{?python_provide:%python_provide python3-asynctest}' [mockbuild@06036e176d6b4146ad180aa2c6c53351 ~]$ (In reply to Carl George from comment #1) > * The summary and description lines should be no longer than 80 characters > [1]. Done. > * Replace fancy single and double quotes (’ and ”) in description with > standard ascii quotes (' and "). Why? SPEC must only be UTF-8 encoded, I don't recall any rules against using proper quoting characters. > * Use the %pypi_source macro to define Source0 [2]. Done. > * Upstream uses setuptools in setup.py [3], so it needs to be listed as a > build requirement. Furthermore, upstream specifically calls out setuptools > >= 30.3.0 [4], so that minimum version requirement should be used with the > build requirement. No. All current Fedora releases and EPEL7 have setuptools newer than 30.3.0. Also, python3-setuptools is pulled in by python3-devel. > * The %python_provides macro should have an argument of python3-asynctest, > not python3-mmtf. Fixed in -2 already. > * Capitalize your changelog entry. Why? The changelog entries are not sentences. > * Instead of using the working directory for the tests, use the buildroot's > sitelib by setting PYTHONPATH to %{buildroot}%{python3_sitelib}. Done. https://rathann.fedorapeople.org/review/python-asynctest/python-asynctest.spec https://rathann.fedorapeople.org/review/python-asynctest/python-asynctest-0.12.2-3.fc30.src.rpm * Mon Jan 14 2019 Dominik Mierzejewski <dominik> 0.12.2-3 - remove trailing space from description - rename pname to standard pypi_name and use pypi_source macro
> Why? SPEC must only be UTF-8 encoded, I don't recall any rules against using > proper quoting characters. > Why? The changelog entries are not sentences. I thought these things were in the guidelines somewhere, but I can't find it now so I must have been mistaken. My personal preferences aside, keep the fancy quotes and lowercase if you like. > My reading of the above is that it's not necessary if subpackage is named > python3-%{pypi_name}. "In order to make the switch from Python 2 to Python 3 automatic, all packages that provide python3-%{srcname} (for any %{srcname}) SHOULD use the %python_provide macro with the package name" [0] It currently evaluates to an empty string with a python3-* argument, but in the future the macro definition will change so that python3-asynctest provides python-asynctest. As you can see it is a SHOULD item, so it's not strictly a blocker, but if you leave it out you are adding to the manual work that must eventually be done when unversioned python becomes python3 distribution-wide. > No. All current Fedora releases and EPEL7 have setuptools newer than 30.3.0. > Also, python3-setuptools is pulled in by python3-devel. "MUST: All build dependencies must be listed in BuildRequires" [1] Just because something happens to get installed in the build root doesn't mean you don't need to list it. The project's setup.py file imports setuptools, so it's a build requirement. You don't have to include the minimum version, but there is no reason not to since upstream has been courteous enough to mention it. Thanks for taking care of the test suite stuff, I'm able to run fedora-review now. Please take care of this rpmlint error [2]: python3-asynctest.noarch: E: summary-too-long C Enhance the standard unittest package with features for testing asyncio libraries [0]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_the_python_provide_macro [1]: https://docs.fedoraproject.org/en-US/packaging-guidelines/ReviewGuidelines/ [2]: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_summary_and_description
https://rathann.fedorapeople.org/review/python-asynctest/python-asynctest.spec https://rathann.fedorapeople.org/review/python-asynctest/python-asynctest-0.12.2-4.fc30.src.rpm * Wed Jan 16 2019 Dominik Mierzejewski <dominik> 0.12.2-4 - reintroduce python_provide macro call - put python3-setuptools into BR, even though it's redundant now - shorten Summary: to fit in 80 characters
Package approved.
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/python-asynctest
python-asynctest-0.12.2-4.fc29 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2019-10e14bde0e
python-asynctest-0.12.2-4.fc29 has been pushed to the Fedora 29 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-10e14bde0e
python-asynctest-0.12.2-4.fc29 has been pushed to the Fedora 29 stable repository. If problems still persist, please make note of it in this bug report.