Bug 1623220

Spec URL: https://sgallagh.fedorapeople.org/packagereview/python-mmdzanata/python-mmdzanata.spec
SRPM URL: https://sgallagh.fedorapeople.org/packagereview/python-mmdzanata/python-mmdzanata-0.5-1.fc29.src.rpm
COPR Build: https://copr.fedorainfracloud.org/coprs/sgallagh/mmdzanata/

This version properly includes the COPYING file referenced in the source header comments.

* I don't see the need for that runtime dependency on setuptools.
  What's the point?

* Missing runtime dependency on python3-gobject-base or equivalent.

* All files state "Copyright (C) 2017-2018 Stephen Gallagher", yet the
  first upstream commit is from a week ago.

* Does cli.py need the shebang line?

* As an executable, mmdzanata could have a manpage.

(In reply to Petr Šabata from comment #2)
> * I don't see the need for that runtime dependency on setuptools.
>   What's the point?

That was added automatically by pyp2rpm; it's probably not needed.

> 
> * Missing runtime dependency on python3-gobject-base or equivalent.

Ah, right. Good catch.

> 
> * All files state "Copyright (C) 2017-2018 Stephen Gallagher", yet the
>   first upstream commit is from a week ago.

Oops, copy-paste error.

> 
> * Does cli.py need the shebang line?

It does not need one, it's not meant to be directly executable. It's wrapped by console_scripts in setup.py.

> 
> * As an executable, mmdzanata could have a manpage.

It probably should, but I haven't written one yet and this isn't a blocker for initial review IMHO.

Spec URL: https://sgallagh.fedorapeople.org/packagereview/python-mmdzanata/python-mmdzanata.spec
SRPM Url: https://sgallagh.fedorapeople.org/packagereview/python-mmdzanata/python-mmdzanata-0.6-1.fc29.src.rpm
Copr Build: https://copr.fedorainfracloud.org/coprs/sgallagh/mmdzanata/package/python-mmdzanata/

This review now depends on BZ #1623469 to provide the tool needed to generate the requested manpages. I've dropped the setuptools runtime dep, added python3-gobject-base, fixed the copyright headers, dropped the shebang line and added the manpage.

Spec URL: https://sgallagh.fedorapeople.org/packagereview/python-mmdzanata/python-mmdzanata.spec
SRPM URL: https://sgallagh.fedorapeople.org/packagereview/python-mmdzanata/python-mmdzanata-0.6-2.fc29.src.rpmCopr Build: https://copr.fedorainfracloud.org/coprs/sgallagh/mmdzanata/package/python-mmdzanata/

Previous build failed missing some BuildRequires. This fixes that.

* I still see the runtime dep on setuptools.

* Build deps on click-man and pygobject are listed twice.

* I suppose this method of manpage generation loads the files, so that's
  why you need all those build deps.  Is that right?  In that case you
  should also add python(requests) as a build dep.

* I confirm the shebang and copyright changes.

Spec URL: https://sgallagh.fedorapeople.org/packagereview/python-mmdzanata/python-mmdzanata.spec
SRPM URL: https://sgallagh.fedorapeople.org/packagereview/python-mmdzanata/python-mmdzanata-0.7-1.fc29.src.rpm
COPR Build: https://copr.fedorainfracloud.org/coprs/sgallagh/mmdzanata/package/python-mmdzanata/


I fixed the runtime dep on setuptools.

Yes, the manpage generation needed the builddeps to function. I'm not sure why it didn't need requests, but I've added it for future-proofing.

I replaced most of the runtime deps with the
%{?python_enable_dependency_generator}
macro, but left the pygobject one because that can't be present in setup.py (I don't think) and therefore auto-detected.

I added a python2 subpackage to support pungi in more places.

Almost there!

%{description} isn't an expandable macro.  If you want to re-use the description text, define something else and use that.  The current result is you have the literal "%{description}" as your package description.

Also not sure whether you need line 74, doesn't the next step overwrite it?
Though I haven't tried that, just thinking.

Spec URL: https://sgallagh.fedorapeople.org/packagereview/python-mmdzanata/python-mmdzanata.spec
SRPM URL: https://sgallagh.fedorapeople.org/packagereview/python-mmdzanata/python-mmdzanata-0.7-2.fc29.src.rpm
COPR Build: https://copr.fedorainfracloud.org/coprs/sgallagh/mmdzanata/package/python-mmdzanata/

I fixed the description. Regarding the binary directory purge, that's future-proof in case for some reason the python2 build ever produces different content from the python3 one. It was put there by pyp2rpm and I don't see a good reason to remove it.

Ack.  Approving.

(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/python-mmdzanata

This package was approved and imported in repositories and it was later retired, but this review ticket was never closed.
I'm closing it now.