[Bug 980222] Review Request: perl-Class-Accessor-Classy - Accessors with minimal inheritance

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=980222

Petr Šabata <psabata@xxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|fedora-review?              |fedora-review+

--- Comment #3 from Petr Šabata <psabata@xxxxxxxxxx> ---
(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.

-- 
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=g9nvGHjTT3&a=cc_unsubscribe
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]