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

fix #65: Added message back to user when the path given does not exist #95

Merged
merged 6 commits into from Sep 26, 2013

Conversation

chafnan
Copy link
Contributor

@chafnan chafnan commented Sep 22, 2013

No description provided.

if (!exists(program.path)) {
print("PhantomJS does not exist at '" + program.path + "'\n");
process.exit(1)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wont this keep the other ways of finding the phantomjs bin below this?
https://github.com/metaskills/mocha-phantomjs/blob/master/bin/mocha-phantomjs#L96-L114

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my opinion, it's about dependancy.

If a user depends on a different version of PhantomJS then what is installed in the bin, then that user will be expecting that specific path to be used. The user probably has a specific reason for using the path argument as it is optional.

For example, what happens if the user misspells the path and the bin version is used. It could fail and the time the user spends debugging could result in time lost. By just failing the program and telling the user that the path is incorrect, they will get immediate feedback. Resulting is no time lost for misspelling the path to PhantomJS.

@metaskills
Copy link
Collaborator

Hmmm, I forgot that w now have a --path argument via ticket #64. When I think about it, that is what $PATH is for and if someone want to use a different bin, then they should symlink the existing bin or change their path as needed.

I think this ticket exposes more work once we took on that responsibility, sigh. Anyways, maybe we should just warn if the path argument is not found and continue searching. Then if no phantomsjs is found, we exit with an error.

-p, --path <path>            path to PhantomJS binary

@chafnan
Copy link
Contributor Author

chafnan commented Sep 24, 2013

Changing the symlink to the bin would be annoying if you had two projects that use different versions of PhantomJS. On the the other hand, failing the app if the path argument is wrong could be too dramatic. I think a warning would be fine.

I updated the code and tests to just send a warning to the stderr.

metaskills added a commit that referenced this pull request Sep 26, 2013
fix #65: Added message back to user when the path given does not exist
@metaskills metaskills merged commit 08f57df into nathanboktae:master Sep 26, 2013
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.

None yet

2 participants