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=747674 --- Comment #5 from Jose Pedro Oliveira <jpo@xxxxxxxxxxxx> 2011-11-17 01:20:18 EST --- Parag, Thanks for the review. I've a couple of comments regarding your suggestions below: (In reply to comment #2) ---[snip]--- > > Suggestions: > 1) BR: on perl is really not needed Removed. > 2) If building this on Fedora only then you don't need > a) buidlroot > b) cleaning of buildroot in %install > c) %clean section > d) defattr(-,root,root,-) I will most likely apply these changes in a future release. Right now I'm also interested in having this perl module in the EPEL5/6 repositories (at least EPEL6), but it may prove to problematic (the Test::More version is one of the problems). > 3) you don't need following explicit Requires: as they will get automatically > pulled by yum > Requires: perl(JSON) >= 2.00 > Requires: perl(Task::Weaken) I don't think these two are pulled: 1) JSON is pulled in by a require statement and this is not caught by rpmbuild; 2) Task::Weaken needs to be explicity pulled in order to handle some (old?) Scalar::Util problems Regards, jpo -- 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