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

[Red Hat Bugzilla]

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

Bug ID: 980222
           Summary: Review Request: perl-Class-Accessor-Classy - Accessors
                    with minimal inheritance
           Product: Fedora
           Version: rawhide
         Component: Package Review
          Severity: medium
          Priority: medium
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail],
                    [e-mail]

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-1.fc17.src.rpm

Description: The Class::Accessor::Classy Perl module provides an extremely
small footprint accessor/mutator declaration scheme for fast and convenient
object attribute setup. It is intended as a simple and speedy mechanism for
preventing hash-key typos rather than a full-blown object system with type
checking and so on.

The accessor methods appear as a hidden parent class of your package and
generally try to stay out of the way. The accessors and mutators generated are
of the form C<foo()> and C<set_foo()>, respectively.

This software is licensed under the same terms as Perl itself (GPL+ or
Artistic), so there should not be any problems there.

This package is a pre-requisite for another Perl module that I would like to
package: perl-CAD-Format-STL (which reads and writes stereo lithography format
files used in 3d printing and other computer aided design applications).

Fedora Account System Username: jcp

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

[Red Hat Bugzilla]

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

Petr Šabata  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |[e-mail]
           Assignee|[e-mail]    |[e-mail]
              Flags|                            |fedora-review?

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

[Red Hat Bugzilla]

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

--- Comment #1 from Petr Šabata  ---
Do you plan to push this package into old EPEL repositories?  If not...

Line 37 is obsolete and should be removed:
 37 rm -rf %{buildroot}

So is line 39...:
 39 find %{buildroot} -depth -type d -exec rmdir {} 2>/dev/null ;

And also the whole %clean section and %defattr macro.

You're missing some build-time dependencies:
 - perl; called in spec, recommended
 - perl(attributes), lib/Class/Accessor/Classy.pm:676, recommended
 - perl(Carp), lib/Class/Accessor/Classy.pm:6, required
 - perl(strict), various places, recommended
 - perl(version), t/00-load.t:10, recommended
 - perl(warnings), various places, recommended

Note: Nowadays using simple 'perl' is preferred to the '%{__perl}' macro. 
However, that's up to you.

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

[Red Hat Bugzilla]

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

--- Comment #2 from John C Peterson  ---

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.

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 "/" !!!)

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).

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.

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

[Red Hat Bugzilla]

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

Petr Šabata  changed:

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

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

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

[Red Hat Bugzilla]

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

John C Peterson  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |fedora-cvs?

--- Comment #4 from John C Peterson  ---
New Package SCM Request
=======================
Package Name: perl-Class-Accessor-Classy
Short Description: Accessors with minimal inheritance
Owners: jcp
Branches: f17 f18 f19 el6
InitialCC: perl-sig

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

[Red Hat Bugzilla]

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

Jens Petersen  changed:

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

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

[Red Hat Bugzilla]

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

--- Comment #5 from Jens Petersen  ---
Git done (by process-git-requests).

F17 is in pre-EOL phase so no new packages being added to that branch.

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

[Red Hat Bugzilla]

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

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |MODIFIED

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

[Red Hat Bugzilla]

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

--- Comment #6 from Fedora Update System  ---
perl-Class-Accessor-Classy-0.9.1-2.fc19 has been submitted as an update for
Fedora 19.
https://admin.fedoraproject.org/updates/perl-Class-Accessor-Classy-0.9.1-2.f...

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

[Red Hat Bugzilla]

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

--- Comment #7 from Fedora Update System  ---
perl-Class-Accessor-Classy-0.9.1-2.fc18 has been submitted as an update for
Fedora 18.
https://admin.fedoraproject.org/updates/perl-Class-Accessor-Classy-0.9.1-2.f...

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

[Red Hat Bugzilla]

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

--- Comment #8 from Fedora Update System  ---
perl-Class-Accessor-Classy-0.9.1-2.el6 has been submitted as an update for
Fedora EPEL 6.
https://admin.fedoraproject.org/updates/perl-Class-Accessor-Classy-0.9.1-2.e...

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

[Red Hat Bugzilla]

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

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|MODIFIED                    |ON_QA

--- Comment #9 from Fedora Update System  ---
perl-Class-Accessor-Classy-0.9.1-2.el6 has been pushed to the Fedora EPEL 6
testing repository.

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

[Red Hat Bugzilla]

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

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ON_QA                       |CLOSED
   Fixed In Version|                            |perl-Class-Accessor-Classy-
                   |                            |0.9.1-2.fc18
         Resolution|---                         |ERRATA
        Last Closed|                            |2013-08-02 17:50:38

--- Comment #10 from Fedora Update System  ---
perl-Class-Accessor-Classy-0.9.1-2.fc18 has been pushed to the Fedora 18 stable
repository.

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

[Red Hat Bugzilla]

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

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
   Fixed In Version|perl-Class-Accessor-Classy- |perl-Class-Accessor-Classy-
                   |0.9.1-2.fc18                |0.9.1-2.fc19

--- Comment #11 from Fedora Update System  ---
perl-Class-Accessor-Classy-0.9.1-2.fc19 has been pushed to the Fedora 19 stable
repository.

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

[Red Hat Bugzilla]

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

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
   Fixed In Version|perl-Class-Accessor-Classy- |perl-Class-Accessor-Classy-
                   |0.9.1-2.fc19                |0.9.1-2.el6

--- Comment #12 from Fedora Update System  ---
perl-Class-Accessor-Classy-0.9.1-2.el6 has been pushed to the Fedora EPEL 6
stable repository.