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

correct installdeps docs to note it's following extra rules #529

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

wchristian
Copy link
Contributor

As discussed, this patch corrects the description of installdeps in various places to stop it giving the appearance of installing all dependencies.

Relates to #528

@@ -869,7 +869,7 @@ Options:
-n,--notest Do not run unit tests
--test-only Run tests only, do not install
-S,--sudo sudo to run install commands
--installdeps Only install dependencies
--installdeps Only install 'test' dependencies
Copy link
Owner

Choose a reason for hiding this comment

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

I don' think calling them 'test' is the right wording.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Owner

Choose a reason for hiding this comment

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

No, that's an explanation why test is included in some situations and not in others.

@wchristian
Copy link
Contributor Author

Removed the bit about --tests-only --notest as that ignores the cpanfile and thus doesn't work for dists with configure phase dependencies.

@miyagawa
Copy link
Owner

Removed the bit about --tests-only --notest as that ignores the cpanfile and thus doesn't work for dists with configure phase dependencies.

Well, --tests-only --notest will actually work if you have the right META.json. I get that putting that in --installdeps section might be confusing though.

@wchristian
Copy link
Contributor Author

So, about the names, do you disagree with the wording in the in-code comment then? How would you word it? I'm happy to adjust both the docs further and include that comment too.

@wchristian
Copy link
Contributor Author

wchristian commented Feb 13, 2017

Ah, didn't see the comment in the review bit, sorry. No editing the comment then. Still curious how you'd word that then? It's important to me that any doc talking about it makes it clear that it will skip some of the deps in cpanfile.

@miyagawa
Copy link
Owner

I would call it "runtime/test dependencies" if you want to be specific. The fact "build" is included is an artifact that "test" dependencies only exist in META spec v2.

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.

2 participants