Bug 904328 - Review Request: perl-Config-ApacheFormat - Use Apache format config files
Review Request: perl-Config-ApacheFormat - Use Apache format config files
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Petr Šabata
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-01-26 05:02 EST by Normunds
Modified: 2014-04-03 12:18 EDT (History)
3 users (show)

See Also:
Fixed In Version: perl-Config-ApacheFormat-1.2-1.fc17
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2014-04-03 12:18:15 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
psabata: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Normunds 2013-01-26 05:02:02 EST
Spec URL: http://unibackup.rule.lv/FedoraRPM/perl-Config-ApacheFormat.spec
SRPM URL: http://unibackup.rule.lv/FedoraRPM/perl-Config-ApacheFormat-1.2-1.fc16.src.rpm
Description: This module is designed to parse a configuration file in the same syntax used by
the Apache web server (see http://httpd.apache.org for details). This allows you
to build applications which can be easily managed by experienced Apache admins.
Also, by using this module, you'll benefit from the support for nested blocks
with built-in parameter inheritance. This can greatly reduce the amount or
repeated information in your configuration files.

Fedora Account System Username: normunds
Comment 1 Normunds 2013-01-26 05:19:01 EST
All packages mentioned below were tested with rpmlint, mock (for i386 arch) and koji (16, 17, 18, 19, rawhide). These are my first packages for Fedora, so if you find something to improve in one of them, don't bother, I'll check other packages for reported problems.

Need sponsor.

bug 903824 perl-Convert-Age.spec
bug 903826 perl-Net-Domain-TLD.spec
bug 903829 perl-Time-Interval.spec
bug 904328 perl-Config-ApacheFormat.spec
bug 904329 perl-Data-Validate-Domain.spec
bug 904330 perl-Data-Validate-IP.spec
bug 904331 perl-Shell.spec

Thanks.
Comment 2 Petr Šabata 2013-01-28 08:29:19 EST
Taking the review.
Comment 3 Petr Šabata 2013-01-28 09:01:18 EST
Issues similar to those of other reviews...

Missing build-time dependency:
perl(Carp)

Unneeded runtime dependencies:
perl(Class::MethodMaker)
perl(File::Spec)
perl(Scalar::Util)
perl(Text::Balanced)
Those are automatically picked up by rpmbuild.

Lines 40 and 41 are unneeded.  None of the files has executable bits set.
Line 52 is also unneeded (done by rpmbuild).

And the command macros could be replaced with simple calls.
Comment 4 Normunds 2013-01-28 17:02:31 EST
Spec file and src package updated.

 * Fixed dependencies.
 * Replaced macros with simple commands.
 * Removed useless lines

New package overwritten in original location.
Comment 5 Petr Šabata 2013-01-29 09:54:05 EST
Removing FE-NEEDSPONSOR.
Comment 6 Petr Šabata 2013-01-29 10:31:57 EST
I see you also added the Data::Dumper dependency.  Its only used in the dump() subroutine which is not called from the tests at all.  You may drop this again :)

And again, that %{__perl} macro in MODULE_COMPAT...

Not approving yet.
Comment 7 Normunds 2013-01-31 17:06:25 EST
Moved Data::Dumper to Requires, removed macro. Please check. 

Thanks.
Comment 8 Petr Šabata 2013-02-01 04:38:40 EST
(In reply to comment #7)
> Moved Data::Dumper to Requires

Good.  Indeed, it wasn't autodetected in this case.
perl(Class::MethodMaker), on the other hand, is.  Why did you add it now?

I see you've removed your email address from the changelog here too; why?
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Changelogs
Comment 9 Normunds 2013-02-02 13:57:07 EST
> [...] wasn't autodetected in this case.
> perl(Class::MethodMaker), on the other hand, is.  Why did you add it now?
Still getting used to rpmbuild behavior. I was assuming that if rpmbuild detects Requirement and requirement is specified in spec file, it will show duplicate Requirement in rpm --requires output.

 * perl(Class::MethodMaker) removed from Requirements.
 * Added obfuscated e-mail.

Thanks.
Comment 10 Petr Šabata 2013-02-04 09:22:59 EST
(In reply to comment #9)
> > [...] wasn't autodetected in this case.
> > perl(Class::MethodMaker), on the other hand, is.  Why did you add it now?
> Still getting used to rpmbuild behavior. I was assuming that if rpmbuild
> detects Requirement and requirement is specified in spec file, it will show
> duplicate Requirement in rpm --requires output.

It used to be like that.  Not anymore.  The duplicates get filtered out.

>  * perl(Class::MethodMaker) removed from Requirements.

Ok.

>  * Added obfuscated e-mail.

*sigh* If you must... enclose it with qw(< >) at least.
Comment 11 Normunds 2013-02-04 11:24:34 EST
Spec file/srpm updated, added <> to e-mail.
Comment 12 Normunds 2013-02-05 13:52:26 EST
Another reviewer suggested to use simpler e-mail obfuscation, so, to be consistent, I changed it for all packages.
Comment 13 Petr Šabata 2013-02-15 06:09:25 EST
Alright, approving.
Comment 14 Normunds 2013-02-15 07:41:43 EST
Thanks.

Adding fedora-cvs flag.
Comment 15 Gwyn Ciesla 2013-02-15 08:31:10 EST
No SCM request found, unsetting flag.
Comment 16 Normunds 2013-02-15 08:54:50 EST
Sorry for that. Re-trying.

New Package SCM Request
=======================
Package Name: perl-Config-ApacheFormat
Short Description: Use Apache format config files.
Owners: normunds psabata
Branches: f17 f18
InitialCC: perl-sig
Comment 17 Gwyn Ciesla 2013-02-15 08:58:56 EST
Git done (by process-git-requests).
Comment 18 Fedora Update System 2013-02-15 10:21:16 EST
perl-Config-ApacheFormat-1.2-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/perl-Config-ApacheFormat-1.2-1.fc17
Comment 19 Fedora Update System 2013-02-15 13:38:31 EST
perl-Config-ApacheFormat-1.2-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/perl-Config-ApacheFormat-1.2-1.fc18
Comment 20 Fedora Update System 2013-02-25 21:34:30 EST
perl-Config-ApacheFormat-1.2-1.fc18 has been pushed to the Fedora 18 stable repository.
Comment 21 Fedora Update System 2013-02-25 21:55:12 EST
perl-Config-ApacheFormat-1.2-1.fc17 has been pushed to the Fedora 17 stable repository.

Note You need to log in before you can comment on or make changes to this bug.