Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 464117 (perl-SVN-Notify) - Review Request: perl-SVN-Notify - Perl module for Subversion activity notification
Summary: Review Request: perl-SVN-Notify - Perl module for Subversion activity notific...
Keywords:
Status: CLOSED NOTABUG
Alias: perl-SVN-Notify
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: perl-Text-Trac
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2008-09-26 11:53 UTC by Timon
Modified: 2011-01-10 23:38 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-01-10 23:38:28 UTC


Attachments (Terms of Use)
perl-SVN-Notify.spec (deleted)
2009-06-19 13:44 UTC, Timon
no flags Details
perl-SVN-Notify-2.79-1.fc12.src.rpm (deleted)
2009-06-19 13:45 UTC, Timon
no flags Details

Description Timon 2008-09-26 11:53:55 UTC
Spec URL: http://timon.perm.ru/download/perl-SVN-Notify.spec
SRPM URL: http://timon.perm.ru/download/perl-SVN-Notify-2.78-1.fc9.src.rpm
Description: SVN::Notify is a Perl module for Subversion activity notification.

addititional information here: http://search.cpan.org/dist/SVN-Notify/

Comment 1 Jason Tibbitts 2008-10-03 16:52:04 UTC
I don't see you in the packager group.  What's your FAS ID?  Do you need a sponsor?

Comment 2 Timon 2008-10-04 04:54:56 UTC
yes, i need a sponsor. or somebody can become a packager of this package :)

Comment 3 Jason Tibbitts 2008-10-09 16:10:01 UTC
Well, which is it?  If you're not interested in submitting this package yourself then this ticket should be closed.  Otherwise I'll go ahead and indicate that you need a sponsor.

Comment 4 Timon 2008-10-09 17:33:58 UTC
my FAS ID - timon

Comment 5 Jason Tibbitts 2008-10-16 01:16:08 UTC
This failed to build for me; all of the tests failed due to a lack of Test::More.  It is quite important that you test your builds in mock or do a koji scratch build before submitting them.  To do that, install fedora-packager and then either run
  mock -r fedora-rawhide-i386 --rebuild perl-SVN-Notify-2.78-1.fc9.src.rpm
(which may take quite some time to download packages if you don't have a fast connection) or run fedora-packager-setup to get your keys set up and then run
  koji build --scratch dist-f10 perl-SVN-Notify-2.78-1.fc9.src.rpm
to build the package in our buildsystem on what will soon become F10.

After adding BuildRequires: perl(Test::More), things build OK.  However, I guess this requires Text::Trac in order to install, even though nothing in this ticket indicates that.  I'll set up the proper dependencies, although I wish that had been done ahead of time so that I wouldn't have wasted the time reviewing this.

A couple of other comments:

It looks a little odd to not enclose "_bindir" (after %files -n svnnotify) in brackets, but I don't think it makes any difference.  Otherwise there are minor deviations from our usual Perl packages (which are mostly autogenerated) but nothing material.

The descriptions are a bit sparse.  I guess the one for the Perl module comes from upstream, but the one for the svnnotify package doesn't seem to actually indicate with the package does.

Comment 6 Timon 2008-10-16 06:19:04 UTC
>It looks a little odd to not enclose "_bindir" (after %files -n svnnotify) in
brackets, but I don't think it makes any difference.
fixed

>The descriptions are a bit sparse.
fixed

>However, I guess this requires Text::Trac in order to install
fixed in dep list

>This failed to build for me
fixed. tested with mock

Spec URL: http://timon.perm.ru/download/perl-SVN-Notify.spec
SRPM URL: http://timon.perm.ru/download/perl-SVN-Notify-2.78-2.fc9.src.rpm

Comment 7 Jochen Schmitt 2009-05-04 17:30:12 UTC
1.) What this line should mean?

Requires:       perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))

This may caused, that you have to rebuild your package for every new perl version which will been released.

2.) For what do you need this statements?

find $RPM_BUILD_ROOT -type d -depth -exec rmdir {} 2>/dev/null \;

chmod -R u+rwX,go+rX,go-w $RPM_BUILD_ROOT/*

Keep in mind, that <ou can set file permissions in the %files stanza.

It may be nice to subsitute

Requires:       perl-SVN-Notify


into

Requires:       perl-SVN-Notify = %{version}-%{release}

The scratch build on koji works fine. I will sponsor you, if we can finished this review.

Comment 8 Jochen Schmitt 2009-05-04 17:39:52 UTC
I have done some examinations. The results are:

1.) We should remove the following lines:

find $RPM_BUILD_ROOT -type d -depth -exec rmdir {} 2>/dev/null \;

chmod -R u+rwX,go+rX,go-w $RPM_BUILD_ROOT/*

2.) The descriptions for the svnnotify contains a line with more the
79 characters. This will be complainted by rpmlint.

Comment 9 Jochen Schmitt 2009-05-04 17:48:36 UTC
I have found out, that a new release 2.79 is available now.

So if you can provide a new package based on this release and fixed the issues described on comment #7 and #8, we can start an official review of your package.

Comment 10 Timon 2009-06-19 13:43:01 UTC
>Requires:       perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))
http://fedoraproject.org/wiki/Packaging:Perl
All perl modules must include the versioned MODULE_COMPAT Requires:
Requires:  perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))
This is to ensure that perl packages have a dependency on a perl which provides the appropriate versioned directory structure (otherwise, the modules won't be found). 

>Requires:       perl-SVN-Notify = %{version}-%{release}
fixed

> We should remove the following lines:
>find $RPM_BUILD_ROOT -type d -depth -exec rmdir {} 2>/dev/null \;
>chmod -R u+rwX,go+rX,go-w $RPM_BUILD_ROOT/*
fixed

>The descriptions for the svnnotify contains a line with more the 79 characters.
fixed

Comment 11 Timon 2009-06-19 13:44:02 UTC
Created attachment 348665 [details]
perl-SVN-Notify.spec

Comment 12 Timon 2009-06-19 13:45:13 UTC
Created attachment 348666 [details]
perl-SVN-Notify-2.79-1.fc12.src.rpm

Comment 13 Timon 2009-06-19 13:45:55 UTC
>I have found out, that a new release 2.79 is available now.
fixed

Comment 14 Jason Tibbitts 2011-01-10 23:38:28 UTC
No response after pings in another of the submitter's tickets; closing them all.


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