Bug 1400236 - Review Request: python-podcastparser - Simplified, fast RSS parsing library
Summary: Review Request: python-podcastparser - Simplified, fast RSS parsing library
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Shawn Iwinski
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1400141
TreeView+ depends on / blocked
 
Reported: 2016-11-30 17:39 UTC by Gwyn Ciesla
Modified: 2017-01-09 01:20 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2017-01-09 00:57:48 UTC
Type: Bug
Embargoed:
shawn: fedora-review+


Attachments (Terms of Use)
fedora-review.txt (7.78 KB, text/plain)
2016-12-29 09:11 UTC, Shawn Iwinski
no flags Details

Comment 1 Dhanesh B. Sabane 2016-12-06 17:19:19 UTC
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?

Comment 2 Zbigniew Jędrzejewski-Szmek 2016-12-06 17:37:52 UTC
(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?

Comment 3 Dhanesh B. Sabane 2016-12-06 18:03:24 UTC
(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.

Comment 4 Zbigniew Jędrzejewski-Szmek 2016-12-06 18:51:36 UTC
(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

Comment 5 Dhanesh B. Sabane 2016-12-06 18:59:44 UTC
(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. :)

Comment 8 Shawn Iwinski 2016-12-29 09:11:13 UTC
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

Comment 9 Shawn Iwinski 2016-12-29 09:11:55 UTC
No Blockers.

===== APPROVED =====

Comment 10 Gwyn Ciesla 2016-12-29 13:36:19 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/python-podcastparser

Comment 11 Fedora Update System 2016-12-29 13:51:37 UTC
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

Comment 12 Fedora Update System 2016-12-29 13:51:51 UTC
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

Comment 13 Gwyn Ciesla 2016-12-29 13:53:00 UTC
Thank you!

Comment 14 Fedora Update System 2016-12-31 09:26:57 UTC
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

Comment 15 Fedora Update System 2016-12-31 09:51:10 UTC
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

Comment 16 Fedora Update System 2017-01-09 00:57:48 UTC
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.

Comment 17 Fedora Update System 2017-01-09 01:20:16 UTC
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.


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