This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 464117 - (perl-SVN-Notify) Review Request: perl-SVN-Notify - Perl module for Subversion activity notification
Review Request: perl-SVN-Notify - Perl module for Subversion activity notific...
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
Depends On: perl-Text-Trac
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2008-09-26 07:53 EDT by Timon
Modified: 2011-01-10 18:38 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-01-10 18:38:28 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
perl-SVN-Notify.spec (2.57 KB, text/plain)
2009-06-19 09:44 EDT, Timon
no flags Details
perl-SVN-Notify-2.79-1.fc12.src.rpm (82.75 KB, application/octet-stream)
2009-06-19 09:45 EDT, Timon
no flags Details

  None (edit)
Description Timon 2008-09-26 07:53:55 EDT
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 12:52:04 EDT
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 00:54:56 EDT
yes, i need a sponsor. or somebody can become a packager of this package :)
Comment 3 Jason Tibbitts 2008-10-09 12:10:01 EDT
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 13:33:58 EDT
my FAS ID - timon
Comment 5 Jason Tibbitts 2008-10-15 21:16:08 EDT
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 02:19:04 EDT
>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 13:30:12 EDT
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 13:39:52 EDT
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 13:48:36 EDT
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 09:43:01 EDT
>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 09:44:02 EDT
Created attachment 348665 [details]
perl-SVN-Notify.spec
Comment 12 Timon 2009-06-19 09:45:13 EDT
Created attachment 348666 [details]
perl-SVN-Notify-2.79-1.fc12.src.rpm
Comment 13 Timon 2009-06-19 09:45:55 EDT
>I have found out, that a new release 2.79 is available now.
fixed
Comment 14 Jason Tibbitts 2011-01-10 18:38:28 EST
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.