| Summary: | Review Request: python-flufl-bounce - Email bounce detectors | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Aurelien Bompard <aurelien> |
| 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+
|
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2016-10-18 11:30:35 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
|
Description
Aurelien Bompard
2016-09-14 12:29:38 UTC
Same comments as for the other review, see https://bugzilla.redhat.com/show_bug.cgi?id=1376001#c1. Also, don't repeat the %description, it makes the spec file much harder to read than necessary: %global _description \ The flufl.bounce library provides a set of heuristics and an API for \ detecting the original bouncing email addresses from a bounce message. Many\ formats found in the wild are supported, as are VERP and RFC 3464 (DSN). %description -n %{py2_namespace}-%{srcname} %{_description} etc. In %files, please enumerate the items under %{pythonX_sitelib}/ explicitly. Something like %{python2_sitelib}/libname %{python2_sitelib}/libname-%{version}.egg-info (In reply to Zbigniew Jędrzejewski-Szmek from comment #1) > Same comments as for the other review, see > https://bugzilla.redhat.com/show_bug.cgi?id=1376001#c1. > > Also, don't repeat the %description, it makes the spec file much harder to > read than necessary I renamed the package, and used a macro for summary and description. Nice, I did not know one could have multiline text with backslashes :-) > In %files, please enumerate the items under %{pythonX_sitelib}/ explicitly. > Something like > %{python2_sitelib}/libname > %{python2_sitelib}/libname-%{version}.egg-info Hmm, this does not look like what the template python file suggests: https://fedoraproject.org/wiki/Packaging:Python#Example_common_spec_file Are you sure? What would be the reason to explicitely list them? Spec URL: https://abompard.fedorapeople.org/reviews/flufl.bounce/python-flufl-bounce.spec SRPM URL: https://abompard.fedorapeople.org/reviews/flufl.bounce/python-flufl-bounce-2.3-1.fc23.src.rpm (In reply to Aurelien Bompard from comment #2) > (In reply to Zbigniew Jędrzejewski-Szmek from comment #1) > > In %files, please enumerate the items under %{pythonX_sitelib}/ explicitly. > > Something like > > %{python2_sitelib}/libname > > %{python2_sitelib}/libname-%{version}.egg-info > > Hmm, this does not look like what the template python file suggests: > https://fedoraproject.org/wiki/Packaging:Python#Example_common_spec_file > Are you sure? What would be the reason to explicitely list them? Indeed. I'm a bit surprised that the template does not suggest that. I'll file a ticket for FPC later. The justification for the explicit patterns: 1. when reading the spec file (during review or at any later time), you don't need to *guess* what module name is provided 2. it catches some common errors: - for example, some packages by mistake install tests as a top level-module - some packages install multiple top-level modules unexpectedly - sometimes the python2 and python3 paths gets mixed up 3. those patterns don't require updating if done properly, but you'll get notified if something changes in the package. For an example, see https://bugzilla.redhat.com/show_bug.cgi?id=1375962. It uses: %{python2_sitelib}/%{srcname}-%{version}*-py%{python2_version}.egg-info/ %{python2_sitelib}/%{srcname}/ %{python3_sitelib}/%{srcname}-%{version}*-py%{python3_version}.egg-info/ %{python3_sitelib}/%{srcname}/ Same as in the other package, you can replace the second and subsequent Summary texts with %{summary}, no need to repeat. /usr/lib/python3.5/site-packages/flufl/bounce/NEWS.rst → this should be in %doc /usr/lib/python3.5/site-packages/flufl/bounce/README.rst → this one is repeated from %doc /usr/lib/python3.5/site-packages/flufl/bounce/docs/using.rst → another candidate for %doc conf.py which is for sphinx gets installed as flufl.bounce.conf, you probably want to delete that to avoid confusion and spurious dependencies. In general, you should fix the installation to remove documentation and the related sphinx config from site-packages dirs. The package is essentially OK, but please submit another version, I'll do one more check of paths and provides/requires. Thanks again for the review, I made those changes. Spec URL: https://abompard.fedorapeople.org/reviews/flufl.bounce/python-flufl-bounce.spec SRPM URL: https://abompard.fedorapeople.org/reviews/flufl.bounce/python-flufl-bounce-2.3-1.fc23.src.rpm I still see /usr/lib/python2.7/site-packages/flufl/bounce/conf.py, /usr/lib/python3.5/site-packages/flufl/bounce/conf.py, but OK. + package name is OK + latest version + license is acceptable (LGPLv3+) + license is specified correctly + python packaging template is used + %python_provide is used + provides/requires are OK + builds and installs OK Package is APPROVED. Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/python-flufl-bounce python-flufl-bounce-2.3-1.fc25 has been pushed to the Fedora 25 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-2016-1e2d07997a python-flufl-bounce-2.3-1.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report. |