Bug 1375999 - Review Request: python-flufl-bounce - Email bounce detectors
Summary: Review Request: python-flufl-bounce - Email bounce detectors
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:
TreeView+ depends on / blocked
 
Reported: 2016-09-14 12:29 UTC by Aurelien Bompard
Modified: 2016-10-18 11:30 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-10-18 11:30:35 UTC
Type: ---
zbyszek: fedora-review+


Attachments (Terms of Use)

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.


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