Bug 980222
| Summary: | Review Request: perl-Class-Accessor-Classy - Accessors with minimal inheritance | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | John C Peterson <jcp> |
| Component: | Package Review | Assignee: | Petr Šabata <psabata> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | notting, package-review, psabata |
| Target Milestone: | --- | Flags: | psabata:
fedora-review+
petersen: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | perl-Class-Accessor-Classy-0.9.1-2.el6 | Doc Type: | Bug Fix |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2013-08-02 21:50:38 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
John C Peterson
2013-07-01 18:52:30 UTC
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.
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. (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. 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 Git done (by process-git-requests). F17 is in pre-EOL phase so no new packages being added to that branch. 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.fc19 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.fc18 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.el6 perl-Class-Accessor-Classy-0.9.1-2.el6 has been pushed to the Fedora EPEL 6 testing repository. perl-Class-Accessor-Classy-0.9.1-2.fc18 has been pushed to the Fedora 18 stable repository. perl-Class-Accessor-Classy-0.9.1-2.fc19 has been pushed to the Fedora 19 stable repository. perl-Class-Accessor-Classy-0.9.1-2.el6 has been pushed to the Fedora EPEL 6 stable repository. |