Skip to content
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

Document options #51

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Document options #51

wants to merge 2 commits into from

Conversation

Grinnz
Copy link

@Grinnz Grinnz commented Jul 17, 2018

Not sure if the options supported by cpanm should be documented here, but it should be documented somewhere, and I don't know how it can be documented in cpanm (since cpanm currently does not describe cpanfile usage).

@miyagawa
Copy link
Owner

I think this is not the right place, since cpanfile is a document format and how to interpret is a client-specific behavior.

@Grinnz
Copy link
Author

Grinnz commented Jul 17, 2018

Regardless it should be documented here that it can take options; do you have any idea where cpanm's interpretation can be documented?

@karenetheridge
Copy link

karenetheridge commented Jul 17, 2018

IMHO it would be reasonable to document this both here, with the cpanfile spec (under something like "extensions supported by external clients"), and in the cpanm documentation (under the --cpanfile option).

@Grinnz
Copy link
Author

Grinnz commented Jul 18, 2018

Opened miyagawa/cpanminus#579 for an attempt at documenting the more specific part in cpanm, and updated this PR to document the options in general.

@@ -23,6 +23,9 @@ cpanfile - A format for describing CPAN dependencies for Perl applications
recommends 'DBD::SQLite';
};

requires 'App::cpanminus',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that this would be inconsistent with the format as defined on line 44:

requires $module, $version_requirement, %options;

(although that line is already a bit inaccurate, plenty of cpanfile examples in the wild with no $version_requirement provided...)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the description: "When version requirement is omitted, it is assumed that 0 is passed, meaning any version of the module would satisfy the requirement."

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I see the description, and it works well enough ("a bit inaccurate", not "completely wrong") if the $version_requirement is last in the list... anyone implementing just needs my ($module, $version_requirement) = @_; $version_requirement //= 0;, which is standard enough as Perl constructs go.

requires $module, $version_requirement, %options;

Two scalars, followed by a list of key => value pairs. Now sometimes one of those scalars isn't a scalar? How would you write that documentation line if it takes an "optional" version scalar?

requires $module, @version_requirement_list_with_one_or_zero_items, %options;

?

Looks like it'd drag in needless complexity for anyone implementing this or even trying to understand the documentation from the perspective of a Perl programmer who's maybe seen a scalar or two in their time: far simpler to say "if %options are passed, make the version mandatory".

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The options are required to be in pairs, so you just count the number of arguments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants