-
Notifications
You must be signed in to change notification settings - Fork 18
Allow add() method to accept multiple modules. #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
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.
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.
|
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 |
Now I just have to make the code actually behave that way.
|
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. |
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.
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.
|
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. |
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).
|
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. |
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.