Allow add() method to accept multiple modules. #12

Open
wants to merge 13 commits into
from

Projects

None yet

3 participants

@thaljef
thaljef commented Jul 11, 2011

The add() method can now accept a "modules" (plural) argument that is a hasref containing ModuleName => ModuleVersion pairs. This can be used as an alternative to (but not in conjunction with) the "module" (singular) and "version" arguments.

In practice, any non-trivial distribution is going to contain multiple modules. And those modules could all have different version numbers. But the old "module" (singular) and "version" interface did not support that reality. In fact, mcpi[1] worked around this limitation by calling add() for each module in the distribution. Not only did this seem odd, it causes the tar.gz file to be needlessly copied several times.

I would even sugges that the old "module" (singular) and "version" interface sould be deprecated.

I added test cases to cover the new interface. Also added test cases to cover the argument validation logic for the add() method.

And I updated the POD accordingly.

Jeffrey Thal... added some commits Jul 8, 2011
Jeffrey Thalhammer Allow add() method to accept multiple modules.
The add() method can now accept a "modules" (plural) argument
that is a hasref containing ModuleName => ModuleVersion pairs.
This can be used as an alternative to (but not in conjunction
with) the "module" (singular) and version arguments.

In practice, any non-trivial distribution is going to contain
multiple modules.  And those modules could all have different
version numbers.  But the old "module" (singular) and "version"
interface did not support that reality.  In fact, mcpi[1]
worked around this limitation by calling add() for each
module in the distribution.  Not only did this seem odd,
it causes the tar.gz file to be needlessly copied several
times.

I would even sugges that the old "module" (singular) and
"version" interface sould be deprecated.

I added test cases to cover the new interface.  Also added
test cases to cover the argument validation logic for the
add() method.

And I updated the POD accordingly.
9bd5fe8
Jeffrey Thalhammer Fixing option validation logic for add() method. be29a49
Jeffrey Thalhammer Go back to old name for _optionchk()
I decided to just keep the old name for this subroutine,
just to minimize churn.  Although this sub is only
useful for the add() method.
bd90962
Jeffrey Thalhammer Add MYMETA.* to the ignore list. 5556e82
@wchristian
Collaborator

Just sounding off here, because right now my internet sucks so much that doing anything online is a pain. After the 18th i'll be able to poke at this more closely. Also, magnificent-tears did a patch that seems like it relates to this one: wchristian#2

Jeffrey Thalhammer Modified POD to reflect my preferred interface.
Now I just have to make the code actually behave that way.
8ab6489
@thaljef
thaljef commented Jul 11, 2011

I hate to throw away my own work, but if package discovery was part of the API, then you wouldn't need my modifications for calling add() with multiple modules. In fact, I think you should probably discourage people from giving an explicit module/version number at all. Doing so just creates an opportunity for error, and there's no good reason to specify something different from what is actually in the META. So IMHO, --all-in-meta should be the default operating mode.

Jeffrey Thal... added some commits Jul 11, 2011
Jeffrey Thalhammer Improvements on the add() interface.
You may now specify the version argument with the modules (plural)
argument.  This serves as the default version of any modules
that you did not specify an explicit version number for.  This
is convenient for module authors (like me) who assign the same
version number to all modules in a particular release.
e3b50ac
Jeffrey Thalhammer The --module option is now awesome.
The --module option for mcpi[1] now takes MODULE_NAME=MODULE_VERSION
arguments and can be repeated for multiple modules.  If you do not
specify a MODULE_VERSION for any given MODULE_NAME, then the value
of the --version option is used as the default.  Here are some
examples:

  # Adds Foo-1.2 and Bar-2.3
  mcpi --add --module Foo=1.2 Bar=2.3 --author ME --file Distro.tar.gz

  # Adds Foo-1.2 and Bar-4.1
  mcpi --add --module Foo=1.2 Bar --version 4.1 --author ME --fle Distro.tar.gz

  # Adds Foo-5.2 and Bar-5.2
  mcpi --add --module Foo --module Bar --version 5.2 --author ME --file Distro.tar.gz

Notice that this is still perfectly compatible with the old
style of using --module and --version to add a single module.
27a71b1
Jeffrey Thalhammer POD edits. 5af9d18
@thaljef
thaljef commented Jul 11, 2011

I've improved on the add() interface a bit more, allowing the 'version' option to be used as a default, if you don't specify a explicit version for each module.

I also modified the interface to mcpani[1] in a similar fashion. The --add action can now take multiple --module arguments. Each of these is a NAME=VERSION string. The VERSION is optional, so if you don't give it, then the value from --version is used.

Jeffrey Thalhammer The module argument to add() is now polymorphic.
Instead of having separate module (singular) and
modules (plural) arguments, the module argument
is now polymorphic, and will accept either a
stirng (for one module) or a hashref (for
multiple modules).
b108e9b
@thaljef
thaljef commented Jul 11, 2011

Ok, I think this branch is finally ready to pull. Rather than having a separate module (singular) and modules (plural) argument for the add() method, the module (singular) argument is polymorphic and accepts either a module name (as a string) or a hashref of NAME => VERSION pairs. This preserves the existing interface nicely and is quite perlish, IMHO.

However, if the package discovery ("module" discovery, actually) becomes part of the CPAN::Mini::Inject API as magnificent-tears has suggested, then my whole fork is a lot less important. But for those who wish still wish to manually declare which modules/versions are being injected, I believe I've put together a nice interface here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment