Bug 239193 (Module-ExtractUse)

- Summary could be improved, how about appending " in Perl code" to it?

- Rationale for including the t/ dir in docs?  Doesn't look useful to me,
  neither does TODO.

- The included cpan.pl example doesn't work (non-blocker, but nice if fixed):
  $ perl cpan.pl SGML-Parser-OpenSP
  Required option 'allow' is not provided for CPANPLUS::Backend::search by
  ANON at cpan.pl line 12

- perl-Pod-Strip's dependencies would be nice to have fixed before importing
  this, see bug 239241 (no changes needed in this package).  Otherwise:
  $ perl -MModule::ExtractUse -e ''
Base class package "Pod::Simple" is empty.
    (Perhaps you need to 'use' the module which defines that package first.)
 at /usr/lib/perl5/vendor_perl/5.8.8/Pod/Strip.pm line 6
BEGIN failed--compilation aborted at
/usr/lib/perl5/vendor_perl/5.8.8/Pod/Strip.pm line 6.
Compilation failed in require at
/usr/lib/perl5/vendor_perl/5.8.8/Module/ExtractUse.pm line 6.
BEGIN failed--compilation aborted at
/usr/lib/perl5/vendor_perl/5.8.8/Module/ExtractUse.pm line 6.
Compilation failed in require.
BEGIN failed--compilation aborted.

(In reply to comment #1)
> - Summary could be improved, how about appending " in Perl code" to it?

Not a problem.

> - Rationale for including the t/ dir in docs?  Doesn't look useful to me,
>   neither does TODO.

This was actually discussed a little over at bug 235790.  Basically, it boils
down to:
* tests can make good documentation (sometimes better than the actual docs)
* people might actually want to test the package post-installation
* it doesn't hurt anything :)

> - The included cpan.pl example doesn't work (non-blocker, but nice if fixed):
>   $ perl cpan.pl SGML-Parser-OpenSP
>   Required option 'allow' is not provided for CPANPLUS::Backend::search by
>   ANON at cpan.pl line 12

I'll take a look at this -- so long as I don't have to introduce any new deps it
would be nice to have it working :)

> - perl-Pod-Strip's dependencies would be nice to have fixed before importing
>   this, see bug 239241 (no changes needed in this package).  Otherwise:
>   $ perl -MModule::ExtractUse -e ''
> Base class package "Pod::Simple" is empty.
>     (Perhaps you need to 'use' the module which defines that package first.)
>  at /usr/lib/perl5/vendor_perl/5.8.8/Pod/Strip.pm line 6
> BEGIN failed--compilation aborted at
> /usr/lib/perl5/vendor_perl/5.8.8/Pod/Strip.pm line 6.
> Compilation failed in require at
> /usr/lib/perl5/vendor_perl/5.8.8/Module/ExtractUse.pm line 6.
> BEGIN failed--compilation aborted at
> /usr/lib/perl5/vendor_perl/5.8.8/Module/ExtractUse.pm line 6.
> Compilation failed in require.
> BEGIN failed--compilation aborted.

Ugh, messy.  I suppose I could add a requires on perl(Pod::Simple) until it's
fixed in perl-Pod-Strip; for the purposes of review at least.

It's a beautiful day out here in San Francisco, and I'm being informed that I
need to get outside :)  I'll post an updated srpm later on...

* Mon May 07 2007 Chris Weyl <cweyl.edu> 0.19-2
- enhanced %%summary
- fix examples/cpan.pl

SRPM URL: http://home.comcast.net/~ckweyl/perl-Module-ExtractUse-0.19-2.fc6.src.rpm
SPEC URL: http://home.comcast.net/~ckweyl/perl-Module-ExtractUse.spec

(In reply to comment #2)
> (In reply to comment #1)
> 
> > - Rationale for including the t/ dir in docs?  Doesn't look useful to me,
> >   neither does TODO.

TODO is still included in -2.

> * tests can make good documentation (sometimes better than the actual docs)

I don't think that's the case in this package.

> * people might actually want to test the package post-installation

I'm pretty sure that people who install software from rpms don't expect to be
able to do that, at least when not installing a *-test subpackage.  I'm not
saying it's necessarily a doomed idea, but should be discussed in distro wide
context before starting to apply in packages here and there.

> * it doesn't hurt anything :)

* It does add files to the package payload that the overwhelming vast majority
  does not have use for.
* Test suites are also often quick and dirty code which is not meant to be
  used as an example of anything.
* Test suite code is not restricted to using the public API of the software;
  some features can be better tested using knowledge of module internals.  We
  don't want anyone to use such code as an example how to use the packaged 
  software.
* Test suites have dependencies that the users need to take care of manually,
  or the package will be dependency bloated.  Both are bad, and easily solved
  by not including the test suite.

To summarize, I think including test suite code is acceptable if there's an
upstream recommendation to use it as an example, upstream installs it too,
and/or packaging test suites becomes a standard packaging practice (or there's a
decision/consensus that things should move towards that).

Apologies for taking so long -- it's been a busy week and I wanted to make sure
I put appropriate thought into this.

(In reply to comment #4)
> TODO is still included in -2.

This is something I'd prefer to leave in, so I don't inadvertently leave it out
in a future release.  I don't have a particularly strong feeling about it,
however, so I'll drop it.

> > * tests can make good documentation (sometimes better than the actual docs)
> 
> I don't think that's the case in this package.

I actually found t/80_failing.t quite useful in understanding that the parser
will not correctly pick up either 'use constant' or 'use base' constructs.  This
isn't in the documentation, and saved me some pain.
 
> > * people might actually want to test the package post-installation
> 
> I'm pretty sure that people who install software from rpms don't expect to be
> able to do that, at least when not installing a *-test subpackage.  I'm not
> saying it's necessarily a doomed idea, but should be discussed in distro wide
> context before starting to apply in packages here and there.

Well, it's processes like this that allow us to have the experience to be able
to comment on such distro-wide proposals.  I know I'm varying from the beaten
path here, but it seems like an avenue worth exploring, and am willing to be
flexible as we go along.
 
> > * it doesn't hurt anything :)
> 
> * It does add files to the package payload that the overwhelming vast majority
>   does not have use for.

Technically speaking, 99% of what's in %doc right now the "vast majority" has no
use for.  Heck, even the minority of fedora packagers probably don't have much
use for it.

> * Test suites are also often quick and dirty code which is not meant to be
>   used as an example of anything.

Sometimes.  But some of them are quite excellent -- see, e.g., Moose,
Class::MOP, POE, WWW::Myspace, etc -- and even the "quick and dirty ones" serve
their purpose.

> * Test suite code is not restricted to using the public API of the software;
>   some features can be better tested using knowledge of module internals.  We
>   don't want anyone to use such code as an example how to use the packaged 
>   software.

I'm not making any judgements as to how people should or shouldn't use software.
 If someone wants to go off and do things the Wrong Way, then a) it doesn't
matter if the test suite is installed or not and b) they're going to do it
anyways.  Enabling people to test their installed s/w isn't going to impact that.

> * Test suites have dependencies that the users need to take care of manually,
>   or the package will be dependency bloated.  Both are bad, and easily solved
>   by not including the test suite.

Hm.  To date, I've been filtering any additional dependencies the suites
introduce....  I'm not entirely convinced that this is a bad thing, but it
sounds like an argument for a -test subpackage.

> To summarize, I think including test suite code is acceptable if there's an
> upstream recommendation to use it as an example, upstream installs it too,
> and/or packaging test suites becomes a standard packaging practice (or there's a
> decision/consensus that things should move towards that).

Upstream actually has recommended it in several cases I know (Moose being one),
and others it just makes sense.  I also believe, given the small amount of
additional work needed to enable it, enabling people to test installed software
with the _same_ test suite we used during build is a Good Thing.

Also, as spot pointed out in an email in fedora-perl-devel recently, "Blanket
policies help me sleep easier at night."  I'd rather package test suites up
consistently rather than make a subjective judgment call as to their "worth".

I know this is a relatively new idea, but I think it's worth exploring.  It's
flexible processes (like the review process) that allow us to gain experience
with various approaches along these lines, rather than waiting/expecting a
directive to be handed down from upon high.

I would like to see public discussion on fedora-packaging about installing test
suite code that upstream does not install before approving this package.

While I don't think it's a good thing to do usually in the first place, and even
less so in the main package, I won't try to block this one as long as there are
no guidelines about it - so if someone wants to have this package in before the
above discussion takes place or there's a conclusion, feel free to reassign and
complete the review.

Seven months have passed and I haven't seen the discussion I asked for and I
still don't think including t/* is a good idea, so I'm setting this review back
to unassigned.

*** This bug has been marked as a duplicate of 426530 ***