Bug 477953 - Review Request: podcatcher - Armangil's podcast client for the command line
Review Request: podcatcher - Armangil's podcast client for the command line
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-12-26 06:09 EST by Christof Damian
Modified: 2009-01-15 03:09 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-01-15 03:09:36 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Christof Damian 2008-12-26 06:09:13 EST
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 16:23:36 EST
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 16:58:40 EST
(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 05:24:06 EST
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 11:33:30 EST
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 08:54:24 EST
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 11:58:50 EST
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 15:04:08 EST
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 09:11:39 EST
Okay.

-----------------------------------------------------
    This package (podcatcher) is APPROVED by mtasaka
-----------------------------------------------------
Comment 9 Christof Damian 2009-01-11 09:22:47 EST
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 12:16:01 EST
cvs done.
Comment 11 Mamoru TASAKA 2009-01-14 10:09:25 EST
Please rebuild your package also on F-11.
Comment 12 Christof Damian 2009-01-14 16:05:22 EST
(In reply to comment #11)
> Please rebuild your package also on F-11.

that is done.
Comment 13 Mamoru TASAKA 2009-01-15 03:09:36 EST
Okay, thanks.

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