Bug 477953 - Review Request: podcatcher - Armangil's podcast client for the command line
Summary: Review Request: podcatcher - Armangil's podcast client for the command line
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-12-26 11:09 UTC by Christof Damian
Modified: 2009-01-15 08:09 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-01-15 08:09:36 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Christof Damian 2008-12-26 11:09:13 UTC
Spec URL: http://rpms.damian.net/SPECS/podcatcher.spec
SRPM URL: http://rpms.damian.net/SRPMS/podcatcher-3.1.4-1.fc10.src.rpm
Description: Armangil's podcatcher is a podcast client for the command line. It provides several download strategies (new shows only, back-catalog allowed, etc), offers cache management, supports BitTorrent, and generates playlists for media player applications.

Comment 1 Fabian Affolter 2008-12-27 21:23:36 UTC
Just some comments on your spec file

- 'Source0:' should point to the upstream tarball if possible. 
- Please preserve the time stamp in the %install section
- Shouldn't '%doc demo' be '%doc demo/' ?
- Replace '/usr/bin/' with a macro
  https://fedoraproject.org/wiki/PackageMaintainers/CreatingPackageHowTo#Macros

rpmlint is not quite...

[fab@laptop024 SRPMS]$ rpmlint podcatcher-3.1.4-1.fc10.src.rpm 
podcatcher.src: E: no-cleaning-of-buildroot %install
podcatcher.src: W: more-than-one-%changelog-section
podcatcher.src: W: mixed-use-of-spaces-and-tabs (spaces: line 4, tab: line 16)
1 packages and 0 specfiles checked; 1 errors, 2 warnings.

Comment 2 Christof Damian 2008-12-27 21:58:40 UTC
(In reply to comment #1)
> Just some comments on your spec file
> 
> - 'Source0:' should point to the upstream tarball if possible. 

fixed, though the URL has to be looked up again on every release because it contains some release id

> - Please preserve the time stamp in the %install section

fixed

> - Shouldn't '%doc demo' be '%doc demo/' ?

I changed it. I didn't realize that it makes a difference, but it is easier readable. 

> - Replace '/usr/bin/' with a macro
>   https://fedoraproject.org/wiki/PackageMaintainers/CreatingPackageHowTo#Macros

OK.

> rpmlint is not quite...
> 
> [fab@laptop024 SRPMS]$ rpmlint podcatcher-3.1.4-1.fc10.src.rpm 
> podcatcher.src: E: no-cleaning-of-buildroot %install
> podcatcher.src: W: more-than-one-%changelog-section
> podcatcher.src: W: mixed-use-of-spaces-and-tabs (spaces: line 4, tab: line 16)
> 1 packages and 0 specfiles checked; 1 errors, 2 warnings.

those are now fixed too.

I have uploaded the new files.

Comment 3 Fabian Affolter 2009-01-01 10:24:06 UTC
Everytime you make changes in your spec file, you need to bump the release. https://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs

BTW, happy new year

Comment 4 Christof Damian 2009-01-01 16:33:30 UTC
I was wondering about that, but I thought it wasn't necessary, because the SPEC is not even approved yet. I bumped the release now:

Spec URL: http://rpms.damian.net/SPECS/podcatcher.spec
SRPM URL: http://rpms.damian.net/SRPMS/podcatcher-3.1.4-2.fc10.src.rpm

Comment 5 Christof Damian 2009-01-03 13:54:24 UTC
I just reread the join process page and noticed that I forgot to mention that this is my first package and I need a sponsor. I added the FE-NEEDSPONSOR to the block list now.

I also have some other packages, which I will submit once I have a sponsor.

Comment 6 Mamoru TASAKA 2009-01-05 16:58:50 UTC
Well, a very simple package.

(In reply to comment #2)
> (In reply to comment #1)
> > Just some comments on your spec file
> > 
> > - 'Source0:' should point to the upstream tarball if possible. 
> 
> fixed, though the URL has to be looked up again on every release because it
> contains some release id

For rubyforge hosted tarball, I usually use
--------------------------------------------------------------
%define repoid <some number>

Source0: http://rubyforge.org/frs/download.php/%{repoid}/%{name}-%{version}.tar.gz
--------------------------------------------------------------

Then:
-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few (or no) work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on:
http://fedoraproject.org/PackageReviewStatus/NEW.html
(NOTE: please don't choose "Merge Review")


Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------

Comment 7 Christof Damian 2009-01-05 20:04:08 UTC
I have changed the spec to use your repoid idea. I also removed another unused macro:

Spec URL: http://rpms.damian.net/SPECS/podcatcher.spec
SRPM URL: http://rpms.damian.net/SRPMS/podcatcher-3.1.4-3.fc10.src.rpm

-----------

Regarding the sponsoring: I have submitted another package for review: 478877

I will have a look at packages which I might be able to pre-review.

Comment 8 Mamoru TASAKA 2009-01-11 14:11:39 UTC
Okay.

-----------------------------------------------------
    This package (podcatcher) is APPROVED by mtasaka
-----------------------------------------------------

Comment 9 Christof Damian 2009-01-11 14:22:47 UTC
New Package CVS Request
=======================
Package Name: podcatcher
Short Description: Armangil's podcast client for the command line
Owners: cdamian
Branches: F-10 EL-5
InitialCC:

Comment 10 Kevin Fenzi 2009-01-11 17:16:01 UTC
cvs done.

Comment 11 Mamoru TASAKA 2009-01-14 15:09:25 UTC
Please rebuild your package also on F-11.

Comment 12 Christof Damian 2009-01-14 21:05:22 UTC
(In reply to comment #11)
> Please rebuild your package also on F-11.

that is done.

Comment 13 Mamoru TASAKA 2009-01-15 08:09:36 UTC
Okay, thanks.


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