Spec URL: http://mcpierce.fedorapeople.org/rpms/perl-qpid_proton.spec SRPM URL: http://mcpierce.fedorapeople.org/rpms/perl-qpid_proton-0.3-1.fc18.src.rpm Description: Perl language bindings for Qpid Proton. Fedora Account System Username: mcpierce
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