[xen-tools] Re: More changes

[Steve Kemp]

On Wed Sep 05, 2007 at 07:52:08 -0700, C.J. Adams-Collier wrote:

> Yes.  I agree.  Dependencies are often difficult to deal with.

  Especially for non-Debian distributions I find.. but that could
 just be bias. I think that other users are more used to using CPAN
 directly.

> However, based on my review of your code, using Moose could reduce
> your codebase size by nearly 40% and will make it much easier to
> extend.

  In general thats pretty good.  But specifically it's a little
 hard to think of ways the code *should* be extended.  Most of it
 is complete, if you see what I mean?

> I see where you're coming from.  However, as a rule, people who read
> Perl code expect to see a Makefile.PL or Build.PL, a MANIFEST file and
> a META.yml in the root of the distribution.  The same thing is done in
> all modern CPAN code.  I argue that providing the standard
> distribution layout increases, not decreases readability.

  Sure.  My objection is that the fact the code is in perl is
 irrelevent to 99% of users, it is an implementation detail.

  People install the software because they want to create new
 xen guests easily.  So ultimately they only need 'make install',
 and probably 97% of them don't touch anything else.

  If we could have a system that would work for both of us but
 would also keep the top-level clean I'd be extremely happy.

  Even if that ended up being:

    Makefile:
        cpan:
            cp ./misc/Build.pl .
            cp ./misc/MANIFEST .
            perl ./Build.pl && make && make install


> But as I mentioned before, I am happy to maintain an external patch
> that brings the distribution into conformance.

  We'll leave it at that for the moment then.

> Swig makes it possible to call perl from C, but it doesn't make it
> easy to define classes in Perl.  Historically, there has been a lot of
> hand waving and behind-the-curtain hiding involved with OO perl.
> Moose cleans a lot of this up.

  Good catch.

> Yipe!  Your example has no type checking and provides no
> encapsulation.  

  True on type checking, but that could be added if required.  As
 for encapsulation with a couple of getter/setter methods it is
 good enough...

> =head2 logpath
> 
>  Attribute which indicates the log directory.  Defaults to /var/log/xen-tools
> 
> =cut
> 
> has 'logpath'  => ( is      => 'ro',
>                                isa     => 'Str',
>                                default => '/var/log/xen-tools'
>                                );
> 
> EOF

  That just seems alien to the way I think about and read perl.  It's
 too much like magic, and not enough like simple to modify and extend
 for me.

> Yep.  We should also use Devel::Cover to ensure that all use cases are
> accounted for.

  Agreed.

> Okay.  I'll review and provide a patch so you can see whether the
> convenience, simplicity and maintainability outweigh the cost of
> having another external dependency.

  I'm 60/40 against at the moment to be honest.  But at the same
 time I don't want you to stop sending code..

Steve
--