Bug 1002319 - Review Request: perl-Term-ShellUI - Fully-featured shell-like command line environment
Review Request: perl-Term-ShellUI - Fully-featured shell-like command line e...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Christopher Meng
Fedora Extras Quality Assurance
:
: 1018871 (view as bug list)
Depends On:
Blocks: 1002324
  Show dependency treegraph
 
Reported: 2013-08-28 17:53 EDT by Matias Kreder
Modified: 2014-03-03 10:25 EST (History)
5 users (show)

See Also:
Fixed In Version: perl-Term-ShellUI-0.92-2.fc19
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-10-22 23:31:04 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
i: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Matias Kreder 2013-08-28 17:53:08 EDT
Spec URL: http://fedorapeople.org/~delete/kpcli/perl-Term-ShellUI.spec
SRPM URL: http://fedorapeople.org/~delete/kpcli/perl-Term-ShellUI-0.92-1.fc20.src.rpm
Description: 
Term::ShellUI uses the history and autocompletion features of
Term::ReadLine to present a sophisticated command-line interface to the
user. It tries to make every feature that one would expect to see in a
fully interactive shell trivial to implement. You simply declare your
command set and let ShellUI take care of the heavy lifting.

Fedora Account System Username: delete

This RPM was built as a dependency of another package that I'm planning to upload. I need to be sponsored as a packager. 
 
Koji build output: 

[makerpm@builder ~]$ koji build --scratch f19 rpmbuild/SRPMS/perl-Term-ShellUI-0.92-1.fc20.src.rpm 
Uploading srpm: rpmbuild/SRPMS/perl-Term-ShellUI-0.92-1.fc20.src.rpm
[====================================] 100% 00:00:03  33.41 KiB   8.81 KiB/sec
Created task: 5866968
Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=5866968
Watching tasks (this may be safely interrupted)...
5866968 build (f19, perl-Term-ShellUI-0.92-1.fc20.src.rpm): open (buildvm-20.phx2.fedoraproject.org)
  5866969 buildArch (perl-Term-ShellUI-0.92-1.fc20.src.rpm, noarch): open (buildvm-03.phx2.fedoraproject.org)
  5866969 buildArch (perl-Term-ShellUI-0.92-1.fc20.src.rpm, noarch): open (buildvm-03.phx2.fedoraproject.org) -> closed
  0 free  1 open  1 done  0 failed
5866968 build (f19, perl-Term-ShellUI-0.92-1.fc20.src.rpm): open (buildvm-20.phx2.fedoraproject.org) -> closed
  0 free  0 open  2 done  0 failed

5866968 build (f19, perl-Term-ShellUI-0.92-1.fc20.src.rpm) completed successfully
Comment 1 Christopher Meng 2013-08-28 22:08:35 EDT
Remove obsoleted things I've mentioned in another bug of you.

New issue:

I can see:

BuildRequires:  perl-Test-Simple

in you spec.

You must remind that we use this style:

perl(Test::Simple)

Furthermore, let's look at perl-Test-Simple itself.

Its Provides:
perl(Test::Builder) = 0.98
perl(Test::Builder::IO::Scalar) = 2.110
perl(Test::Builder::Module) = 0.98
perl(Test::Builder::Tester) = 1.23
perl(Test::Builder::Tester::Color) = 1.23
perl(Test::Builder::Tester::Tie)
perl(Test::More) = 0.98
perl(Test::Simple) = 0.98
perl-Test-Simple = 0.98.05-3.fc20
perl-Test-Simple-tests = 0.98.05-3.fc20
Comment 2 Matias Kreder 2013-08-29 16:47:17 EDT
I have uploaded a new .spec and .srpm file. 

Spec: http://fedorapeople.org/~delete/kpcli/perl-Term-ShellUI.spec
SRPM: http://fedorapeople.org/~delete/kpcli/perl-Term-ShellUI-0.92-2.fc20.src.rpm

Please let me know if there is anything else you see wrong.
Comment 3 Christopher Meng 2013-08-29 21:13:09 EDT
From the spec I think is good now, however I can't sponsor you so I have to stop here.

Suggestion:

When the sponsor ask you to change something, please sort %install :

make pure_install DESTDIR=$RPM_BUILD_ROOT
find $RPM_BUILD_ROOT -type f -name .packlist -exec rm -f {} \;
%{_fixperms} $RPM_BUILD_ROOT/*

Don't leave too many blank lines.
Comment 4 Michael Schwendt 2013-08-30 05:27:04 EDT
> Don't leave too many blank lines.

Style (and also ordering of tags) inside the spec file is up to the package maintainer. We don't force packagers to adapt to some mandatory style and not to the available spec templates either.

[...]

> Fully-featured

I think it's "full-featured" or "fully featured".


Other than that, I think both %summary and %description don't make clear that this is just a Perl module to develop with and no ready-to-use executable. Even the first sentence of the description is misleading/ambiguous, IMO.

> Summary:        Fully-featured shell-like command line environment

> %description
> Term::ShellUI uses the history and autocompletion features of
> Term::ReadLine to present a sophisticated command-line interface to the
> user. It tries to make every feature that one would expect to see in a
> fully interactive shell trivial to implement. You simply declare your
> command set and let ShellUI take care of the heavy lifting.

Any ideas how to improve it?

  Summary: Full-featured shell-like command line environment API

Or to copy from the description:

  Summary: Make it easy to implement a full-featured shell-like command line environment

  Summary: Perl module to implement a full-featured shell-like command line environment

The %description could simply be reordered to do a better job:

%description
Term::ShellUI is a Perl module, which tries to make every feature that
one would expect to see in a fully interactive shell trivial to implement. 
It uses the history and autocompletion features of Term::ReadLine to present
a sophisticated command-line interface to the user. You simply declare your
command set and let ShellUI take care of the heavy lifting.

[...]

Just my point of view. Anyone search for a "shell" would see this package in the search results.
Comment 5 Matias Kreder 2013-08-30 12:18:14 EDT
Thank you all for the suggestions, I agree that the .spec file looks better without the white lines. Regarding the description and summary, I just had left what the author had uploaded to cpan.org, but it is more clear if we implement your suggestion. 
 
I've re-uploaded the .spec file and the src.rpm to the same location. 

Spec: http://fedorapeople.org/~delete/kpcli/perl-Term-ShellUI.spec
SRPM: http://fedorapeople.org/~delete/kpcli/perl-Term-ShellUI-0.92-2.fc20.src.rpm
Comment 6 Matías Kreder 2013-09-11 18:47:18 EDT
I no longer need sponsorship but this package needs a reviewer.
Comment 7 Christopher Meng 2013-09-11 21:08:46 EDT
[?]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by:
     /usr/share/perl5/vendor_perl/Text(perl-Text-ParseWords, perl-Text-Diff,
     perl-Text-Glob, perl-Text-FindIndent, perl-Text-Autoformat, perl-Text-
     Unidecode, perl-Text-Reform), /usr/share/perl5/vendor_perl/Term(perl-
     Term-UI)
Comment 8 Matías Kreder 2013-09-12 17:16:34 EDT
$ rpm -qlp ../RPMS/noarch/perl-Term-ShellUI-0.92-2.fc21.noarch.rpm
/usr/share/doc/perl-Term-ShellUI
/usr/share/doc/perl-Term-ShellUI/Changes
/usr/share/doc/perl-Term-ShellUI/README
/usr/share/man/man3/Term::ShellUI.3pm.gz
/usr/share/man/man3/Text::Shellwords::Cursor.3pm.gz
/usr/share/perl5/vendor_perl/Term
/usr/share/perl5/vendor_perl/Term/ShellUI.pm
/usr/share/perl5/vendor_perl/Text
/usr/share/perl5/vendor_perl/Text/Shellwords
/usr/share/perl5/vendor_perl/Text/Shellwords/Cursor.pm

I can edit the .spec to package only the .pm files that this packages provide as this: 

%{perl_vendorlib}/Term/*
%{perl_vendorlib}/Text/*

But what would happen if %{perl_vendorlib}/Term or %{perl_vendorlib}/Text directories are not created because they are owned by one of these packages and they are not installed. Add one of them as a dependency might not be correct as they might not be required by the user.
Comment 10 Matias Kreder 2013-09-18 20:56:12 EDT
I red the Wiki page several times and trying to match some of the examples to this case I only get to the conclusion that this package should also own these directories. Based on the rule of thumb described there, I believe that none of the other packages that own this directory should be required as they don't need to be present for the primary functionality of this package. 
If you disagree please let me know and I will fix it.
Comment 11 Christopher Meng 2013-09-25 10:56:18 EDT
Michael, any suggestions?
Comment 12 Michael Schwendt 2013-09-25 15:05:26 EDT
What answer do you hope for? Matias' conclusion in comment 10 is entirely correct. It's exactly the reason why that section of the packaging guidelines exists.
Comment 13 Christopher Meng 2013-09-26 00:08:45 EDT
OK.

APPROVED.
Comment 14 Matías Kreder 2013-10-08 16:13:35 EDT
New Package SCM Request
=======================
Package Name: perl-Term-ShellUI
Short Description: Perl module to implement a full-featured shell-like command line environment
Owners: delete echevemaster
Branches: f18 f19 f20 el5 el6
InitialCC:
Comment 15 Matías Kreder 2013-10-08 16:14:57 EDT
New Package SCM Request
=======================
Package Name: perl-Term-ShellUI
Short Description: Perl module to implement a full-featured shell-like command line environment
Owners: delete echevemaster
Branches: f18 f19 f20 el5 el6
InitialCC: perl-sig

I forget to CC perl-sig
Comment 16 Gwyn Ciesla 2013-10-09 08:07:21 EDT
Git done (by process-git-requests).
Comment 17 Fedora Update System 2013-10-10 16:27:03 EDT
perl-Term-ShellUI-0.92-2.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/perl-Term-ShellUI-0.92-2.fc18
Comment 18 Fedora Update System 2013-10-10 16:33:44 EDT
perl-Term-ShellUI-0.92-2.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/perl-Term-ShellUI-0.92-2.fc19
Comment 19 Fedora Update System 2013-10-10 16:45:29 EDT
perl-Term-ShellUI-0.92-2.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/perl-Term-ShellUI-0.92-2.fc20
Comment 20 Fedora Update System 2013-10-10 16:50:39 EDT
perl-Term-ShellUI-0.92-2.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/perl-Term-ShellUI-0.92-2.el6
Comment 21 Fedora Update System 2013-10-11 19:56:44 EDT
perl-Term-ShellUI-0.92-2.fc19 has been pushed to the Fedora 19 testing repository.
Comment 22 Christopher Meng 2013-10-14 19:31:23 EDT
*** Bug 1018871 has been marked as a duplicate of this bug. ***
Comment 23 Fedora Update System 2013-10-22 23:31:04 EDT
perl-Term-ShellUI-0.92-2.fc19 has been pushed to the Fedora 19 stable repository.
Comment 24 Fedora Update System 2013-10-22 23:39:31 EDT
perl-Term-ShellUI-0.92-2.fc18 has been pushed to the Fedora 18 stable repository.
Comment 25 Fedora Update System 2013-10-26 14:28:37 EDT
perl-Term-ShellUI-0.92-2.el6 has been pushed to the Fedora EPEL 6 stable repository.
Comment 26 Fedora Update System 2013-11-10 01:23:05 EST
perl-Term-ShellUI-0.92-2.fc20 has been pushed to the Fedora 20 stable repository.
Comment 27 Matías Kreder 2014-03-03 10:10:30 EST
Package Change Request
======================
Package Name: perl-Term-ShellUI
New Branches: epel7
Owners: delete
Comment 28 Gwyn Ciesla 2014-03-03 10:25:34 EST
Git done (by process-git-requests).

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