https://bugzilla.redhat.com/show_bug.cgi?id=980222
Petr Šabata psabata@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #3 from Petr Šabata psabata@redhat.com --- (In reply to John C Peterson from comment #2)
Spec URL: http://www.eskimo.com/~jcp/perl-Class-Accessor-Classy.spec SRPM URL: http://www.eskimo.com/~jcp/perl-Class-Accessor-Classy-0.9.1-2.fc17.src.rpm
Hi Petr,
Thanks for doing the review.
Glad to help :)
I think I have addressed those issues in release two above.
I was indeed going to push this back to EPEL at some point. I don't like the look of the "rm -rf %{buildroot}" either, so I put those inside ?rhel conditionals. (Heaven forbid if %{buildroot} were "/" !!!)
The conditionals aren't necessary and I'd say they make the spec even uglier. Also the chances of %buildroot being something terribly wrong and the user doing the build having the permissions to do any damage there are really low. And if that happens, it's their own fault ;)
I'm perhaps a little out of touch with building for EPEL, so I wasn't sure about an explicit Buildroot: ... definition. I had already removed the one that cpanspec had put in there. (I was under the impression that's not needed anymore, even for EPEL).
You will need it for EPEL5. https://fedoraproject.org/wiki/EPEL:Packaging
See the other relevant sections of EPEL guidelines too. You won't need much for EPEL6 only.
I'm a bit surprised that cpanspec missed all of those build dependencies. I'll know to perform my own search of the code for missed build dependencies in the future.
The stable versions of cpanspec only look into the META.* files which often don't contain much (or correct) information. I'm currently working on a better, PPI-based dependency detection but peeking at the code will always be the best option.
Anyhow, the package conforms to the guidelines now. Approving.