Bug 1400236

Disclaimer: This is an unofficial review.

* Use the modname macro in the package name too.

* There is no need to declare a separate macro for summary.  Just enter the summary for the first time and then you can use %{summary} everywhere else. 

* Source0 is incorrect.  It should be 

%{url}/archive/%{version}.tar.gz

* The license associated with the source seems to be ISC only.  I don't understand why ASL is required here.  

* The description is too terse.  Could you elaborate a bit?

(In reply to Dhanesh B. Sabane from comment #1)
> Disclaimer: This is an unofficial review.
> 
> * Use the modname macro in the package name too.
Why?

> * There is no need to declare a separate macro for summary.  Just enter the
> summary for the first time and then you can use %{summary} everywhere else. 
> 
> * Source0 is incorrect.  It should be 
> %{url}/archive/%{version}.tar.gz
What do you mean by incorrect? Including the part after # allows the tarball to be renamed (spectool will use the part after the last slash as the output name).

> * The license associated with the source seems to be ISC only.  I don't
> understand why ASL is required here.  
> 
> * The description is too terse.  Could you elaborate a bit?

(In reply to Zbigniew Jędrzejewski-Szmek from comment #2)
> Why?
So that consistency is maintained.  As far as I know, one of the requirements of a good package is that it consistently uses macros.

> What do you mean by incorrect? Including the part after # allows the tarball
> to be renamed (spectool will use the part after the last slash as the output
> name).

Rpmlint output:

Checking: python2-podcastparser-0.6.0-1.fc24.noarch.rpm
          python3-podcastparser-0.6.0-1.fc24.noarch.rpm
          python-podcastparser-0.6.0-1.fc24.src.rpm
python2-podcastparser.noarch: W: invalid-license ASL ISC
python3-podcastparser.noarch: W: invalid-license ASL ISC
python-podcastparser.src: W: invalid-license ASL ISC
python-podcastparser.src: W: invalid-url Source0: https://github.com/gpodder/podcastparser/archive/v0.6.0.tar.gz#/podcastparser-0.6.0.tar.gz HTTP Error 404: Not Found
3 packages and 0 specfiles checked; 0 errors, 4 warnings.

(In reply to Dhanesh B. Sabane from comment #3)
> (In reply to Zbigniew Jędrzejewski-Szmek from comment #2)
> > Why?
> So that consistency is maintained.  As far as I know, one of the
> requirements of a good package is that it consistently uses macros.

Not really. The guidelines say that macros should be used for directory names [1] (and it's "should", not "must", there are various exceptions) and some binaries. Replacing every instance of the package name with a macro just makes it harder to copy&paste the text, or to click on a link to open it, etc.

[1] https://fedoraproject.org/wiki/Packaging:Guidelines#Macros

> python-podcastparser.src: W: invalid-url Source0:
> https://github.com/gpodder/podcastparser/archive/v0.6.0.tar.gz#/

Ah, the "v" shouldn't be there. Source0 should be
%{url}/archive/%{version}.tar.gz#/%{modname}-%{version}.tar.gz

(In reply to Zbigniew Jędrzejewski-Szmek from comment #4)
> Not really. The guidelines say that macros should be used for directory
> names [1] (and it's "should", not "must", there are various exceptions) and
> some binaries. Replacing every instance of the package name with a macro
> just makes it harder to copy&paste the text, or to click on a link to open
> it, etc.
> 
> [1] https://fedoraproject.org/wiki/Packaging:Guidelines#Macros

Roger that!

> > python-podcastparser.src: W: invalid-url Source0:
> > https://github.com/gpodder/podcastparser/archive/v0.6.0.tar.gz#/
> 
> Ah, the "v" shouldn't be there. Source0 should be
> %{url}/archive/%{version}.tar.gz#/%{modname}-%{version}.tar.gz

I learned something new today.  I didn't know the part after # allows the tarball to be renamed.  Thanks for the clarifications. :)

SPRM: https://fedorapeople.org/~limb/review/python-podcastparser/python-podcastparser-0.6.0-2.fc25.src.rpm
SPEC: https://fedorapeople.org/~limb/review/python-podcastparser/python-podcastparser.spec

Updated.

SPRM: https://fedorapeople.org/~limb/review/python-podcastparser/python-podcastparser-0.6.1-1.fc25.src.rpm
SPEC: https://fedorapeople.org/~limb/review/python-podcastparser/python-podcastparser.spec

New upstream.

Created attachment 1235833 [details]
fedora-review.txt

Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review --mock-config fedora-rawhide-x86_64 --bug 1400236
Buildroot used: fedora-rawhide-x86_64
Active plugins: Python, Generic, Shell-api
Disabled plugins: Java, C/C++, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6

No Blockers.

===== APPROVED =====

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/python-podcastparser

python-podcastparser-0.6.1-1.fc24 gpodder-3.9.3-1.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-7fe8bd33bd

python-podcastparser-0.6.1-1.fc25 gpodder-3.9.3-1.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2016-1785c3943c

Thank you!

gpodder-3.9.3-1.fc25, python-podcastparser-0.6.1-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-1785c3943c

gpodder-3.9.3-1.fc24, python-podcastparser-0.6.1-1.fc24 has been pushed to the Fedora 24 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-7fe8bd33bd

gpodder-3.9.3-1.fc25, python-podcastparser-0.6.1-1.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.

gpodder-3.9.3-1.fc24, python-podcastparser-0.6.1-1.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.