Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=906362 Petr Šabata <psabata@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? | Flags| |fedora-review+ --- Comment #4 from Petr Šabata <psabata@xxxxxxxxxx> --- (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. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=YhkYdRqrEU&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review