Bug 1375999

Summary: Review Request: python-flufl-bounce - Email bounce detectors
Product: [Fedora] Fedora Reporter: Aurelien Bompard <aurelien>
Component: Package ReviewAssignee: Zbigniew Jędrzejewski-Szmek <zbyszek>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: 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
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
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).

Fedora Account System Username: abompard

Comment 1 Zbigniew Jędrzejewski-Szmek 2016-10-05 03:43:39 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

Comment 2 Aurelien Bompard 2016-10-05 09:17:00 UTC
(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?

Comment 4 Zbigniew Jędrzejewski-Szmek 2016-10-09 13:45:40 UTC


(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.

Comment 6 Zbigniew Jędrzejewski-Szmek 2016-10-10 01:33:26 UTC
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.

Comment 7 Patrick Uiterwijk 2016-10-11 12:26:31 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/python-flufl-bounce

Comment 8 Fedora Update System 2016-10-14 04:55:54 UTC
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

Comment 9 Fedora Update System 2016-10-18 11:30:35 UTC
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.