Bug 906362

Taking the review.

I don't see the perl bindings listed on http://qpid.apache.org/proton/ and your Source isn't a URL.  Where does this come from?  There's no explanation in the spec file...

You're also missing the following build-deps:
perl(Test::Exception)
perl(Test::More)

Your changelog entry references version 0.03 instead of 0.3.

Tip: Substitute command macros (like %{__perl}) with simple calls (lines 16 and 43).

Tip: Drop line 53, it's unnecessary.

Why are you excluding the cproton_perl.so from the package?  Does it even work without it?...

(In reply to comment #2)
> I don't see the perl bindings listed on http://qpid.apache.org/proton/ and
> your Source isn't a URL.  Where does this come from?  There's no explanation
> in the spec file...
> 
> You're also missing the following build-deps:
> perl(Test::Exception)
> perl(Test::More)

Fixed that, thanks.

> Your changelog entry references version 0.03 instead of 0.3.

Ugh. Muscle memory from packaging Qpid as well. Fixed.
 
> Tip: Substitute command macros (like %{__perl}) with simple calls (lines 16
> and 43).

What's the advantage of replacing the command macros? I'm not opposed to it, but don't understand why. Especially given that the packaging guidelines use them:

https://fedoraproject.org/wiki/Packaging:Perl

> Tip: Drop line 53, it's unnecessary.

Done.
 
> Why are you excluding the cproton_perl.so from the package?  Does it even
> work without it?...

That exclude seems to be a mistake, since cproton_perl.so still is there in the generated RPM. I've removed the exclude since it's no longer necessary.

Updated SPEC:  http://mcpierce.fedorapeople.org/rpms/perl-qpid_proton.spec
Updated SRPM:  http://mcpierce.fedorapeople.org/rpms/perl-qpid_proton-0.3-1.1.fc18.src.rpm
Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=4918012

(In reply to comment #3)
> (In reply to comment #2)
> > I don't see the perl bindings listed on http://qpid.apache.org/proton/ and
> > your Source isn't a URL.  Where does this come from?  There's no explanation
> > in the spec file...
> > 
> > You're also missing the following build-deps:
> > perl(Test::Exception)
> > perl(Test::More)
> 
> Fixed that, thanks.

Ok, that's acceptable...

> > Your changelog entry references version 0.03 instead of 0.3.
> 
> Ugh. Muscle memory from packaging Qpid as well. Fixed.

Ok.

> > Tip: Substitute command macros (like %{__perl}) with simple calls (lines 16
> > and 43).
> 
> What's the advantage of replacing the command macros? I'm not opposed to it,
> but don't understand why. Especially given that the packaging guidelines use
> them:
> 
> https://fedoraproject.org/wiki/Packaging:Perl

Yes, that's a just a tip since it's becoming the new trend; afaic the %{__perl} macro doesn't offer any advantages, just looks ugly.  Hopefully we'll rework the Perl packaging guidelines one day, once (or if) we reach some consensus in Perl SIG...

> > Tip: Drop line 53, it's unnecessary.
> 
> Done.

Ok.

> > Why are you excluding the cproton_perl.so from the package?  Does it even
> > work without it?...
> 
> That exclude seems to be a mistake, since cproton_perl.so still is there in
> the generated RPM. I've removed the exclude since it's no longer necessary.

Interesting.  The * glob above probably ate that.
Anyway, it's better now.


Approving.

(In reply to comment #4)
> Approving.

Thanks for the quick turnaround on the review, Petr! :)

New Package SCM Request
=======================
Package Name: perl-qpid_proton
Short Description: Perl language bindings for Qpid Proton
Owners: mcpierce
Branches: f17 f18 el6
InitialCC: mcpierce

Since this is a perl package, add perl-sig to InitialCC too, please. (resetting the cvs flag)

New Package SCM Request
=======================
Package Name: perl-qpid_proton
Short Description: Perl language bindings for Qpid Proton
Owners: mcpierce
Branches: f17 f18 el6
InitialCC: mcpierce perl-sig

Git done (by process-git-requests).

Thanks everyone!

Package Change Request
======================
Package Name: perl-qpid_proton
New Branches: epel7
Owners: mcpierce

Git done (by process-git-requests).