Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 1363825 - Review Request: perl-LWP-UserAgent-DNS-Hosts - Override LWP HTTP/HTTPS request's host like /etc/hosts
Summary: Review Request: perl-LWP-UserAgent-DNS-Hosts - Override LWP HTTP/HTTPS reques...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jitka Plesnikova
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-08-03 16:27 UTC by Carl George
Modified: 2016-08-16 13:19 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-08-16 13:19:57 UTC
jplesnik: fedora-review+


Attachments (Terms of Use)

Description Carl George 2016-08-03 16:27:10 UTC
Spec URL: https://carlgeorge.fedorapeople.org/perl-LWP-UserAgent-DNS-Hosts/perl-LWP-UserAgent-DNS-Hosts.spec

SRPM URL: https://carlgeorge.fedorapeople.org/perl-LWP-UserAgent-DNS-Hosts/perl-LWP-UserAgent-DNS-Hosts-0.08-1.fc26.src.rpm

Description: LWP::UserAgent::DNS::Hosts is a module to override HTTP/HTTPS request peer addresses that uses LWP::UserAgent.  This module concept was got from LWP::Protocol::PSGI.

Fedora Account System Username: carlgeorge

Comment 1 Jitka Plesnikova 2016-08-08 13:25:51 UTC
Source file is ok
Summary is ok
License is ok
Description is ok
URL and Source0 are ok
All tests passed

$ rpm -qp --requires perl-LWP-UserAgent-DNS-Hosts-0.08-1.fc26.noarch.rpm | sort | uniq -c
      1 perl(:MODULE_COMPAT_5.24.0)
      1 perl(:VERSION) >= 5.8.1
      1 perl(Carp)
      1 perl(LWP::Protocol)
      1 perl(LWP::Protocol::http)
      1 perl(LWP::Protocol::https)
      1 perl(LWP::UserAgent::DNS::Hosts)
      1 perl(Scope::Guard)
      1 perl(parent)
      1 perl(strict)
      1 perl(warnings)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
Binary requires are Ok.

$ rpm -qp --provides perl-LWP-UserAgent-DNS-Hosts-0.08-1.fc26.noarch.rpm | sort | uniq -c
      1 perl(LWP::Protocol::http::hosts)
      1 perl(LWP::Protocol::https::hosts)
      1 perl(LWP::UserAgent::DNS::Hosts) = 0.08
      1 perl-LWP-UserAgent-DNS-Hosts = 0.08-1.fc26
Binary provides are Ok.

$ rpmlint ./perl-LWP-UserAgent-DNS-Hosts*
2 packages and 1 specfiles checked; 0 errors, 0 warnings.
Rpmlint is ok

FIX: Please add missing build requires
  - make (spec file, lines 45, 49, 55)
  - findutils (spec file, line 50) - in case you do not remove the command
  - perl(Carp) - lib/LWP/UserAgent/DNS/Hosts.pm:6
  - perl(strict)
  - perl(warnings)

FIX: Remove all the bundled Module::Install files from ./inc, build-require
  `perl(inc::Module::Install)' and other needed modules from Module::Install
  namespace (locate functions called from Makefile.PL). Or declare all
  build-time dependencies for the bundled Module::Install modules.

TODO: Please replace PERL_INSTALL_ROOT with more common DESTDIR.
TODO: Use plain 'perl' command instead of %{__perl} macro and build-require 'perl'.

Please correct all 'FIX' issues and consider fixing 'TODO' items and
provide new spec file.

Package NOT approved.

Comment 2 Carl George 2016-08-08 21:23:22 UTC
Thanks for the feedback, I'm working on correcting those dings.  One question though.  Regarding "Use plain 'perl' command", the current Perl packaging guidelines [1] require the following line:

> Requires:  perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))

Should the guidelines be updated to s/%{__perl}/perl/ on that requirement?

[1]: https://fedoraproject.org/wiki/Packaging:Perl

Comment 3 Carl George 2016-08-09 21:23:34 UTC
I'll hold off on that TODO item, pending clarification of the Perl packaging guidelines.  Everything else has been fixed, please review.

Spec URL: https://carlgeorge.fedorapeople.org/perl-LWP-UserAgent-DNS-Hosts/perl-LWP-UserAgent-DNS-Hosts.spec

SRPM URL: https://carlgeorge.fedorapeople.org/perl-LWP-UserAgent-DNS-Hosts/perl-LWP-UserAgent-DNS-Hosts-0.08-2.fc26.src.rpm

Comment 4 Jitka Plesnikova 2016-08-12 08:10:09 UTC
> FIX: Please add missing build requires
>   - make (spec file, lines 45, 49, 55)
>   - findutils (spec file, line 50) - in case you do not remove the command
>   - perl(Carp) - lib/LWP/UserAgent/DNS/Hosts.pm:6
>   - perl(strict)
>   - perl(warnings)
Fixed
 
> FIX: Remove all the bundled Module::Install files from ./inc, build-require
>   `perl(inc::Module::Install)' and other needed modules from Module::Install
>   namespace (locate functions called from Makefile.PL). Or declare all
>   build-time dependencies for the bundled Module::Install modules.
FIX: There are missing two build requires
  perl(Module::Install::Metadata) - name, license, all_from, tests, ...
  perl(Module::Install::WriteAll) - WriteAll


> 
> TODO: Please replace PERL_INSTALL_ROOT with more common DESTDIR.
Fixed.


Please add two missing build-requires.

Otherwise package looks good.
Approved

Comment 5 Carl George 2016-08-12 14:28:55 UTC
I just added those last two build requires in a -3 release.

Thanks for the review!

Comment 6 Gwyn Ciesla 2016-08-15 14:24:18 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/perl-LWP-UserAgent-DNS-Hosts


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