Bug 1889705

Summary: Review Request: perl-Test-Net-LDAP - Net::LDAP subclass for testing
Product: [Fedora] Fedora Reporter: Xavier Bachelot <xavier>
Component: Package ReviewAssignee: Jitka Plesnikova <jplesnik>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: jplesnik, package-review
Target Milestone: ---Flags: jplesnik: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: perl-Test-Net-LDAP-0.07-2.fc34 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2020-10-26 09:09:10 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 Xavier Bachelot 2020-10-20 12:33:06 UTC
Spec URL: https://www.bachelot.org/fedora/SPECS/perl-Test-Net-LDAP.spec
SRPM URL: https://www.bachelot.org/fedora/SRPMS/perl-Test-Net-LDAP-0.07-1.fc34.src.rpm
Description:
This module provides some testing methods for LDAP operations, such
as search, add, and modify, where each method is suffixed with either
_ok or _is.
Fedora Account System Username: xavierb

Comment 1 Jitka Plesnikova 2020-10-21 11:38:48 UTC
Source file is ok
Summary is ok
License is ok
Description is ok
URL and Source0 are ok
All tests passed

BuildRequires
FIX: Please add following build-requires:
     perl-interpreter - perl-Test-Net-LDAP.spec:36
     make - perl-Test-Net-LDAP.spec:37
     coreutils - perl-Test-Net-LDAP.spec:45

If you don't want to add the package to EPEL 7, please consider fixing
these 'TODO' items:
TODO: Add "NO_PACKLIST=1 NO_PERLLOCAL=1" to 'perl Makefile.PL'
      If you use option NO_PACKLIST=1, please add version constrain
      to ExtUtils::MakeMaker >= 6.76
      Remove "find $RPM_BUILD_ROOT -type f -name .packlist -exec rm -f {} \;". 
TODO: Use %{make_install} instead of "make pure_install".

If you want to add it to EPEL 7, please do:
TODO: Replace PERL_INSTALL_ROOT with more common DESTDIR.
FIX:  Add a build-require 'findutils' used in spec file.

$ rpm -qp --requires perl-Test-Net-LDAP-0.07-1.fc34.noarch.rpm | sort | uniq -c | grep -v rpmlib
      1 perl(base)
      1 perl(Exporter)
      1 perl(IO::Socket)
      1 perl(:MODULE_COMPAT_5.32.0)
      1 perl(Net::LDAP)
      1 perl(Net::LDAP::Constant)
      1 perl(Net::LDAP::Entry)
      1 perl(Net::LDAP::Filter)
      1 perl(Net::LDAP::FilterMatch)
      1 perl(Net::LDAP::Util)
      1 perl(Scalar::Util)
      1 perl(strict)
      1 perl(Test::Builder)
      1 perl(Test::Net::LDAP)
      1 perl(Test::Net::LDAP::Mixin)
      1 perl(Test::Net::LDAP::Util)
      1 perl(:VERSION) >= 5.6.0
      1 perl(warnings)
FIX: Add following run-requires:
     perl(Net::LDAP::Bind) - lib/Test/Net/LDAP/Mock/Data.pm:181
     perl(Net::LDAP::RootDSE) - lib/Test/Net/LDAP/Mock/Data.pm:92
     perl(Net::LDAP::Search) - lib/Test/Net/LDAP/Mock/Data.pm:257

$ rpm -qp --provides perl-Test-Net-LDAP-0.07-1.fc34.noarch.rpm | sort | uniq -c
      1 perl(Test::Net::LDAP) = 0.07
      1 perl-Test-Net-LDAP = 0.07-1.fc34
      1 perl(Test::Net::LDAP::Mixin)
      1 perl(Test::Net::LDAP::Mock)
      1 perl(Test::Net::LDAP::Mock::Data)
      1 perl(Test::Net::LDAP::Mock::Node)
      1 perl(Test::Net::LDAP::Util)
Binary provides are Ok.

$ rpmlint ./perl-Test-Net-LDAP*
perl-Test-Net-LDAP.noarch: W: spelling-error %description -l en_US ok -> OK, och, pk
perl-Test-Net-LDAP.src: W: spelling-error %description -l en_US ok -> OK, och, pk
2 packages and 1 specfiles checked; 0 errors, 2 warnings.
Rpmlint is ok

TODO: Remove "find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} 2>/dev/null \;". 
      It's not needed.

Otherwise the package is in line with Fedora and Perl packaging guide lines.
Please correct all 'FIX' items, consider fixing 'TODO' items and provide a new spec file.

Resolution:
NOT approved

Comment 2 Xavier Bachelot 2020-10-21 13:33:02 UTC
Thanks for the review Jitka.
I intend to submit for EL7 as well, so I made the fixes accordingly.

Spec URL: https://www.bachelot.org/fedora/SPECS/perl-Test-Net-LDAP.spec
SRPM URL: https://www.bachelot.org/fedora/SRPMS/perl-Test-Net-LDAP-0.07-2.fc34.src.rpm

Comment 3 Jitka Plesnikova 2020-10-21 14:50:48 UTC
> BuildRequires
> FIX: Please add following build-requires:
>      perl-interpreter - perl-Test-Net-LDAP.spec:36
>      make - perl-Test-Net-LDAP.spec:37
>      coreutils - perl-Test-Net-LDAP.spec:45
> FIX: Add a build-require 'findutils' used in spec file.
+BuildRequires:  coreutils
+BuildRequires:  findutils
+BuildRequires:  make
 BuildRequires:  perl-generators
+BuildRequires:  perl-interpreter
Ok.

> If you want to add it to EPEL 7, please do:
> TODO: Replace PERL_INSTALL_ROOT with more common DESTDIR.
-make pure_install PERL_INSTALL_ROOT=$RPM_BUILD_ROOT
+make pure_install DESTDIR=$RPM_BUILD_ROOT
ok.


> TODO: Remove "find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} 2>/dev/null
> \;". 
>       It's not needed.
 find $RPM_BUILD_ROOT -type f -name .packlist -exec rm -f {} \;
-find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} 2>/dev/null \;
Ok.


> FIX: Add following binary requires:
>      perl(Net::LDAP::Bind) - lib/Test/Net/LDAP/Mock/Data.pm:181
>      perl(Net::LDAP::RootDSE) - lib/Test/Net/LDAP/Mock/Data.pm:92
>      perl(Net::LDAP::Search) - lib/Test/Net/LDAP/Mock/Data.pm:257
Not solve, please add them.

Resolution:
Approved

Comment 4 Xavier Bachelot 2020-10-21 16:53:09 UTC
Sorry, I missed this part.
I added:
Requires:       perl(Net::LDAP::Bind) 
Requires:       perl(Net::LDAP::RootDSE) 
Requires:       perl(Net::LDAP::Search)

Comment 5 Gwyn Ciesla 2020-10-21 17:00:32 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/perl-Test-Net-LDAP