Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=808886 --- Comment #6 from Iain Arnell <iarnell@xxxxxxxxx> 2012-04-02 10:30:26 EDT --- (In reply to comment #3) > > == nits == > > - %{?perl_default_filter} isn't really needed by this package, though it > does no harm either I just automatically add it to every package - as you say, no harm. > - I don't know what the use of the %{__perl} macro buys us really Me either. The guidelines state that %{__python} is acceptable, so presumably %{__perl} is too. But in either case, we've got a lot of work if the binaries are moved or renamed. > - There's no need to remove empty directories from the buildroot; even > ancient versions of rpm like the one in Red Hat Linux 9 didn't complain > about them That's good to know. Another anachronism that can be dropped. > In my local build of this package I also run the author/release tests. I > appreciate that it doesn't do much to improve the test coverage of *this* > package, but it does improve the test coverage of the release test packages > that are used to do the testing, resulting in issues like Bug #786849 being > discovered and fixed. Interesting thought. And as a soon-to-be-core module, there's a better than normal chance that the extra tests will continue to work in future. My only concern would be introducing circular dependencies - but a quick repoquery doesn't show anything too worrying. I guess I'll go ahead and do it. > APPROVED. Thanks for the review. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review