https://bugzilla.redhat.com/show_bug.cgi?id=1018859
Bug ID: 1018859 Summary: Review Request: perl-Term-Clui - Term::Clui Perl module Product: Fedora Version: rawhide Component: Package Review Assignee: nobody@fedoraproject.org Reporter: k.georgiou@atreides.org.uk QA Contact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org
Spec URL: http://georgiou.fedorapeople.org//perl-Term-Clui.spec SRPM URL: http://georgiou.fedorapeople.org//perl-Term-Clui-1.68-1.fc19.src.rpm
Description: Term::Clui offers a high-level user interface to give the user of command- line applications a consistent "look and feel". Its metaphor for the computer is as a human-like conversation-partner, and as each question/response is completed it is summarised onto one line, and remains on screen, so that the history of the session gradually accumulates on the screen and is available for review, or for cut/paste. This user interface can therefore be intermixed with standard applications which write to STDOUT or STDERR, such as make, pgp, rcs etc.
https://bugzilla.redhat.com/show_bug.cgi?id=1018859
--- Comment #1 from Kostas Georgiou k.georgiou@atreides.org.uk --- This package built on koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=6058648
https://bugzilla.redhat.com/show_bug.cgi?id=1018859
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |ppisar@redhat.com Assignee|nobody@fedoraproject.org |ppisar@redhat.com Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=1018859
--- Comment #2 from Petr Pisar ppisar@redhat.com --- URL is usable. Ok. Source0 is usable. Ok. Source tar ball is original (SHA-256: 007f1cc9c5779fe9b2bacefec8535157ae647499347ffbbea931da63e793f216). Ok.
FIX: The summary is not descriptive. See README or Clui.pm for better text.
Description verified from Clui.pm. Ok.
License verified from Clui.pm, Clui/FileSelect.pm, test.pl, examples/login_shell, examples/test_script, examples/linux_admin, examples/audio_stuff, examples/choose. The py/test_script is not part of binary package. Ok. No XS code. noarch BuildArch is Ok.
TODO: Remove the BuildRoot definition and cleaning in the %install section and remove the whole %clean section. They are not needed for Fedora.
TODO: You can replace `%{__perl}' macro with plain `perl' command.
TODO: You can replace PERL_INSTALL_ROOT argument with standard DESTDIR argument in the %install section.
TODO: You can remove deleting empty directories in the %install section. ExtUtils::MakeMaker does not create empty directories anymore.
FIX: Remove %defattr macro from %files section. This is not needed anymore (since rpm-4.4).
TODO: You could package the `example' subdirectory as a documentation.
FIX: Build-require `perl(Exporter)' (Clui.pm:13).
TODO: Build-require `perl(strict)' (Clui.pm:20). TODO: Build-require `perl(warnings)' (Clui.pm:20).
The Term::ReadKey, Term::ReadLine::Gnu, Term::Size modules are recommended. Ok.
All tests pass. Ok.
$ rpmlint perl-Term-Clui.spec ../SRPMS/perl-Term-Clui-1.68-1.fc21.src.rpm ../RPMS/noarch/perl-Term-Clui-1.68-1.fc21.noarch.rpm perl-Term-Clui.src: W: spelling-error %description -l en_US summarised -> summarized, summarize perl-Term-Clui.src: W: spelling-error %description -l en_US pgp -> pg, pp, pep perl-Term-Clui.src: W: spelling-error %description -l en_US rcs -> rs, cs, arcs perl-Term-Clui.noarch: W: spelling-error %description -l en_US summarised -> summarized, summarize perl-Term-Clui.noarch: W: spelling-error %description -l en_US pgp -> pg, pp, pep perl-Term-Clui.noarch: W: spelling-error %description -l en_US rcs -> rs, cs, arcs 2 packages and 1 specfiles checked; 0 errors, 6 warnings. rpmlint is Ok.
$ rpm -q -lv -p ../RPMS/noarch/perl-Term-Clui-1.68-1.fc21.noarch.rpm drwxr-xr-x 2 root root 0 Oct 16 12:46 /usr/share/doc/perl-Term-Clui -rw-r--r-- 1 root root 3848 Jul 3 02:59 /usr/share/doc/perl-Term-Clui/Changes -rw-r--r-- 1 root root 3422 Nov 10 2010 /usr/share/doc/perl-Term-Clui/README -rw-r--r-- 1 root root 7715 Oct 16 12:46 /usr/share/man/man3/Term::Clui.3pm.gz -rw-r--r-- 1 root root 3528 Oct 16 12:46 /usr/share/man/man3/Term::Clui::FileSelect.3pm.gz drwxr-xr-x 2 root root 0 Oct 16 12:46 /usr/share/perl5/vendor_perl/Term drwxr-xr-x 2 root root 0 Oct 16 12:46 /usr/share/perl5/vendor_perl/Term/Clui -rw-r--r-- 1 root root 55522 Jul 3 10:48 /usr/share/perl5/vendor_perl/Term/Clui.pm -rw-r--r-- 1 root root 9843 Mar 23 2013 /usr/share/perl5/vendor_perl/Term/Clui/FileSelect.pm File layout and permissions is Ok.
$ rpm -q --requires -p ../RPMS/noarch/perl-Term-Clui-1.68-1.fc21.noarch.rpm | sort -i | uniq -c 1 perl(Exporter) 1 perl(:MODULE_COMPAT_5.18.1) 1 perl(Term::ReadKey) 1 perl(Term::ReadLine::Gnu) 1 perl(Term::Size) 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 TODO: Run-require `perl(strict)' (Clui.pm:20). TODO: Run-require `perl(warnings)' (Clui.pm:20).
$ rpm -q --provides -p ../RPMS/noarch/perl-Term-Clui-1.68-1.fc21.noarch.rpm | sort -i | uniq -c 1 perl(Term::Clui) = 1.68 1 perl-Term-Clui = 1.68-1.fc21 1 perl(Term::Clui::FileSelect) = 1.68 Binary provides are Ok.
$ resolvedeps rawhide ../RPMS/noarch/perl-Term-Clui-1.68-1.fc21.noarch.rpm Binary dependencies resolvable. Ok.
Package builds in F21 (http://koji.fedoraproject.org/koji/taskinfo?taskID=6064821). Ok.
Otherwise the package is in line with Fedora and Perl packaging guidelines.
Please correct all `FIX' items, consider fixing `TODO' items, and provide new SPEC file. Resolution: Package NOT approved.
https://bugzilla.redhat.com/show_bug.cgi?id=1018859
--- Comment #3 from Kostas Georgiou k.georgiou@atreides.org.uk --- Thanks for the review, here are the new files and the changes.
Spec URL: http://georgiou.fedorapeople.org/perl-Term-Clui.spec SRPM URL: http://georgiou.fedorapeople.org/perl-Term-Clui-1.68-2.fc21.src.rpm
FIX: The summary is not descriptive. See README or Clui.pm for better text.
DONE
TODO: Remove the BuildRoot definition and cleaning in the %install section and remove the whole %clean section. They are not needed for Fedora.
DONE, I can always add them back if we need the package in epel-5
TODO: You can replace `%{__perl}' macro with plain `perl' command.
DONE
TODO: You can replace PERL_INSTALL_ROOT argument with standard DESTDIR argument in the %install section.
This doesn't seem to work: make pure_install DISTDIR=/builddir/build/BUILDROOT/perl-Term-Clui-1.68-2.fc21.x86_64 ERROR: Can't create '/usr/share/man/man3' Do not have write permissions on '/usr/share/man/man3'
TODO: You can remove deleting empty directories in the %install section. ExtUtils::MakeMaker does not create empty directories anymore.
DONE
FIX: Remove %defattr macro from %files section. This is not needed anymore (since rpm-4.4).
DONE
TODO: You could package the `example' subdirectory as a documentation.
DONE
FIX: Build-require `perl(Exporter)' (Clui.pm:13). TODO: Build-require `perl(strict)' (Clui.pm:20). TODO: Build-require `perl(warnings)' (Clui.pm:20). TODO: Run-require `perl(strict)' (Clui.pm:20). TODO: Run-require `perl(warnings)' (Clui.pm:20).
DONE
https://bugzilla.redhat.com/show_bug.cgi?id=1018859
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #4 from Petr Pisar ppisar@redhat.com --- Spec file changes: --- perl-Term-Clui.spec.old 2013-10-14 16:30:16.000000000 +0200 +++ perl-Term-Clui.spec 2013-10-16 18:53:57.000000000 +0200 @@ -1,22 +1,26 @@ Name: perl-Term-Clui Version: 1.68 -Release: 1%{?dist} -Summary: Term::Clui Perl module +Release: 2%{?dist} +Summary: Perl module offering a Command-Line User Interface License: GPL+ or Artistic Group: Development/Libraries URL: http://search.cpan.org/dist/Term-Clui/ Source0: http://www.cpan.org/authors/id/P/PJ/PJB/Term-Clui-%%7Bversion%7D.tar.gz -BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) BuildArch: noarch BuildRequires: perl(ExtUtils::MakeMaker) BuildRequires: perl(Term::ReadKey) BuildRequires: perl(Term::ReadLine::Gnu) BuildRequires: perl(Term::Size) BuildRequires: perl(Test::Simple) +BuildRequires: perl(Exporter) +BuildRequires: perl(strict) +BuildRequires: perl(warnings) Requires: perl(Term::ReadKey) Requires: perl(Term::ReadLine::Gnu) Requires: perl(Term::Size) -Requires: perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version)) +Requires: perl(strict) +Requires: perl(warnings) +Requires: perl(:MODULE_COMPAT_%(eval "`perl -V:version`"; echo $version))
%description Term::Clui offers a high-level user interface to give the user of command- @@ -30,33 +34,31 @@
%prep %setup -q -n Term-Clui-%{version} +#Don't pull in the examples dependencies +chmod -x examples/*
%build -%{__perl} Makefile.PL INSTALLDIRS=vendor +perl Makefile.PL INSTALLDIRS=vendor make %{?_smp_mflags}
%install -rm -rf %{buildroot} - make pure_install PERL_INSTALL_ROOT=%{buildroot}
find %{buildroot} -type f -name .packlist -exec rm -f {} ; -find %{buildroot} -depth -type d -exec rmdir {} 2>/dev/null ;
%{_fixperms} %{buildroot}/*
%check make test
-%clean -rm -rf %{buildroot} - %files -%defattr(-,root,root,-) -%doc Changes README +%doc Changes README examples %{perl_vendorlib}/* %{_mandir}/man3/*
%changelog +* Wed Oct 16 2013 Kostas Georgiou georgiou@opengamma.com 1.68-2 +- Review changes/fixes #1018859. + * Wed Oct 02 2013 Kostas Georgiou georgiou@opengamma.com 1.68-1 - Specfile autogenerated by cpanspec 1.78.
FIX: The summary is not descriptive. See README or Clui.pm for better text.
+Summary: Perl module offering a Command-Line User Interface Ok.
TODO: Remove the BuildRoot definition and cleaning in the %install section and remove the whole %clean section. They are not needed for Fedora.
-BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
-rm -rf %{buildroot}
-%clean -rm -rf %{buildroot} Ok.
TODO: You can replace `%{__perl}' macro with plain `perl' command.
Ok.
TODO: You can replace PERL_INSTALL_ROOT argument with standard DESTDIR argument in the %install section.
This doesn't seem to work: make pure_install DISTDIR=/builddir/build/BUILDROOT/perl-Term-Clui-1.68-2.fc21.x86_64 ERROR: Can't create '/usr/share/man/man3' Do not have write permissions on '/usr/share/man/man3'
I guess you misspelled DESTDIR as DISTDIR.
TODO: You can remove deleting empty directories in the %install section. ExtUtils::MakeMaker does not create empty directories anymore.
-find %{buildroot} -depth -type d -exec rmdir {} 2>/dev/null ; Ok.
FIX: Remove %defattr macro from %files section. This is not needed anymore (since rpm-4.4).
-%defattr(-,root,root,-) Ok.
TODO: You could package the `example' subdirectory as a documentation.
-%doc Changes README +%doc Changes README examples Ok.
FIX: Build-require `perl(Exporter)' (Clui.pm:13).
+BuildRequires: perl(Exporter) Ok.
TODO: Build-require `perl(strict)' (Clui.pm:20).
+BuildRequires: perl(strict) Ok.
TODO: Build-require `perl(warnings)' (Clui.pm:20).
+BuildRequires: perl(warnings) Ok.
TODO: Run-require `perl(strict)' (Clui.pm:20).
+Requires: perl(strict) Ok.
TODO: Run-require `perl(warnings)' (Clui.pm:20).
+Requires: perl(warnings) Ok.
$ rpmlint perl-Term-Clui.spec ../SRPMS/perl-Term-Clui-1.68-2.fc21.src.rpm ../RPMS/noarch/perl-Term-Clui-1.68-2.fc21.noarch.rpm perl-Term-Clui.src: W: spelling-error %description -l en_US summarised -> summarized, summarize perl-Term-Clui.src: W: spelling-error %description -l en_US pgp -> pg, pp, pep perl-Term-Clui.src: W: spelling-error %description -l en_US rcs -> rs, cs, arcs perl-Term-Clui.noarch: W: spelling-error %description -l en_US summarised -> summarized, summarize perl-Term-Clui.noarch: W: spelling-error %description -l en_US pgp -> pg, pp, pep perl-Term-Clui.noarch: W: spelling-error %description -l en_US rcs -> rs, cs, arcs 2 packages and 1 specfiles checked; 0 errors, 6 warnings. rpmlint is Ok.
$ rpm -q -lv -p ../RPMS/noarch/perl-Term-Clui-1.68-2.fc21.noarch.rpm drwxr-xr-x 2 root root 0 Oct 17 08:28 /usr/share/doc/perl-Term-Clui -rw-r--r-- 1 root root 3848 Jul 3 02:59 /usr/share/doc/perl-Term-Clui/Changes -rw-r--r-- 1 root root 3422 Nov 10 2010 /usr/share/doc/perl-Term-Clui/README drwxr-xr-x 2 root root 0 Oct 17 08:28 /usr/share/doc/perl-Term-Clui/examples -rw-r--r-- 1 root root 32203 Apr 11 2013 /usr/share/doc/perl-Term-Clui/examples/audio_stuff -rw-r--r-- 1 root root 3256 Sep 27 2009 /usr/share/doc/perl-Term-Clui/examples/choose -rw-r--r-- 1 root root 15558 Mar 18 2010 /usr/share/doc/perl-Term-Clui/examples/linux_admin -rw-r--r-- 1 root root 5824 Mar 10 2012 /usr/share/doc/perl-Term-Clui/examples/login_shell -rw-r--r-- 1 root root 8696 Mar 26 2012 /usr/share/doc/perl-Term-Clui/examples/test_script -rw-r--r-- 1 root root 7715 Oct 17 08:28 /usr/share/man/man3/Term::Clui.3pm.gz -rw-r--r-- 1 root root 3528 Oct 17 08:28 /usr/share/man/man3/Term::Clui::FileSelect.3pm.gz drwxr-xr-x 2 root root 0 Oct 17 08:28 /usr/share/perl5/vendor_perl/Term drwxr-xr-x 2 root root 0 Oct 17 08:28 /usr/share/perl5/vendor_perl/Term/Clui -rw-r--r-- 1 root root 55522 Jul 3 10:48 /usr/share/perl5/vendor_perl/Term/Clui.pm -rw-r--r-- 1 root root 9843 Mar 23 2013 /usr/share/perl5/vendor_perl/Term/Clui/FileSelect.pm File permissions and layout are Ok.
$ rpm -q --requires -p ../RPMS/noarch/perl-Term-Clui-1.68-2.fc21.noarch.rpm | sort -i | uniq -c 1 perl(:MODULE_COMPAT_5.18.1) 1 perl(Exporter) 1 perl(Term::ReadKey) 1 perl(Term::ReadLine::Gnu) 1 perl(Term::Size) 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.
$ resolvedeps rawhide ../RPMS/noarch/perl-Term-Clui-1.68-2.fc21.noarch.rpm Binary dependencies resolvable. Ok.
Package builds in F21 (http://koji.fedoraproject.org/koji/taskinfo?taskID=6069374). Ok.
Package is good.
Please consider replacing the PERL_INSTALL_ROOT with DESTDIR before building this package. Resolution: Package APPROVED.
https://bugzilla.redhat.com/show_bug.cgi?id=1018859
Kostas Georgiou k.georgiou@atreides.org.uk changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
--- Comment #5 from Kostas Georgiou k.georgiou@atreides.org.uk --- New Package SCM Request ======================= Package Name: perl-Term-Clui Short Description: Perl module offering a Command-Line User Interface Owners: georgiou Branches: f18 f19 f20 el6 InitialCC: perl-sig
https://bugzilla.redhat.com/show_bug.cgi?id=1018859
Jon Ciesla limburgher@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-cvs? |fedora-cvs+
https://bugzilla.redhat.com/show_bug.cgi?id=1018859
--- Comment #6 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
https://bugzilla.redhat.com/show_bug.cgi?id=1018859
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |ON_QA
--- Comment #7 from Petr Pisar ppisar@redhat.com --- This package in all Fedora stable repositories.
It only waits in EPEL-6 testsing repository https://admin.fedoraproject.org/updates/FEDORA-EPEL-2013-12001/perl-Term-Clui-1.68-3.el6.
https://bugzilla.redhat.com/show_bug.cgi?id=1018859
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |k.georgiou@atreides.org.uk Flags| |needinfo?(k.georgiou@atreid | |es.org.uk)
--- Comment #8 from Petr Pisar ppisar@redhat.com --- Could you please submit the EPEL-6 build to stable repository?
package-review@lists.fedoraproject.org