On Wed, Apr 4, 2012 at 12:20 PM, Nick Wellnhofer <[email protected]> wrote:
> I posted some initial patches to LUCY-215 (patches 0004 - 0013):
>
> https://issues.apache.org/jira/browse/LUCY-215
>
> I took a simpler approach by using a hashref as config object. Module::Build
> generates very nice accessors for them.
The patch sequence looks great. +1 to commit, with the caveat that I'd like to
relitigate the minimum version for Module::Build. :)
A few notes:
* There are several nice implementation improvements and simplifications! I
particularly enjoyed this one:
0008-LUCY-215-Use-Module-Build-s-include_dirs.patch
* The end product, Clownfish::CFC::Perl::Build has no POD describing its
public API. That's the most important part! You're such a strong coder
that we can have a lot of confidence in your implementations -- but we
need to build consensus about the public interface.
I suppose all the features utilized by the extension Build.PL file you
provided that are not in the Module::Build API constitute the
public API, no?
* We currently require Module::Build version 0.2808_01, which is what
shipped with Perl 5.10.0. I'm reluctant to bump the Module::Build version
number to 0.3404 in order to get c_source accepting multiple directories,
since that would exclude stock installs of Perl 5.10.x and 5.12.x. Can we
remain with 0.2808_01 and hack up something approximating the
clownfish_params property instead of bumping to 0.31 to get
Module::Build#add_property?
* See http://markmail.org/message/hx3nmnu2mtt2t3mb for why we have been
avoiding M::B#dist_version up till now. Man, how I hate Perl's TMTOWTDI
version numbers. :(
I'm OK with this change nevertheless, because the worst it will do is
cause XSLoader to error out because of a mismatch between an X.Y.Z version
in the shared object and an X.YYYZZZ version in Lucy.pm. If that happens,
we'll get a bunch of CPAN testers failures.
* I'd like to do some work on parcels. It looked like you were handling
them as an implementation detail to be generalized, so I don't think we'll
collide.
* It would be nice to get some of the configuration stuff out of this module
and into Charmonizer, e.g. checking for compiler and compiler version and
deriving compiler flags. The flag deriving code has been duplicated in
ruby/Rakefile, and we're going to start paying the price for violating
DRY. It would be nice to continue the work that Joe started last year
consolidating our build systems; I have some ideas about how to go about
this...
Marvin Humphrey
