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

(#137) Nicer response when binary doesn't exist #149

Merged
merged 1 commit into from
Jul 24, 2017

Conversation

rodjek
Copy link
Contributor

@rodjek rodjek commented Jul 13, 2017

As reported in #137, when a user installs the PDK gem on a fresh machine that
does not have git installed, they get an unhelpful message about git failing
with a misleading reference to files inside the PDK.

With this change, we instead check that the binary being called exists before
attempting to execute it and raise an error suggesting that the user needs to
install the appropriate package before trying again.

This won't be a big issue once we have system packages going out, but it
provides a nicer initial experience for users installing the gem.

root@stretch:~# pdk new module foo --skip-interview
pdk (INFO): Creating new module: foo
pdk (FATAL): Unable to find `git`. Check that it is installed and try again.

root@stretch:~# apt-get install -y git
[output trimmed]

root@stretch:~# pdk new module foo --skip-interview
pdk (INFO): Creating new module: foo
root@stretch:~#

Fixes #137

As reported in puppetlabs#137, when a user installs the PDK gem on a fresh machine that
does not have git installed, they get an unhelpful message about git failing
with a misleading reference to files inside the PDK.

With this change, we instead check that the binary being called exists before
attempting to execute it and raise an error suggesting that the user needs to
install the appropriate package before trying again.

This won't be a big issue once we have system packages going out, but it
provides a nicer initial experience for users installing the gem.

```
root@stretch:~# pdk new module foo --skip-interview
pdk (INFO): Creating new module: foo
pdk (FATAL): Unable to find `git`. Check that it is installed and try again.

root@stretch:~# apt-get install -y git
[output trimmed]

root@stretch:~# pdk new module foo --skip-interview
pdk (INFO): Creating new module: foo
root@stretch:~#
```

Fixes puppetlabs#137
@bmjen bmjen added this to the 0.5.0 milestone Jul 13, 2017
bmjen
bmjen previously requested changes Jul 13, 2017
Copy link
Contributor

@bmjen bmjen left a comment

Choose a reason for hiding this comment

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

Going to block merge on this PR until 0.4.0 is tagged.

@@ -26,6 +26,7 @@ Gem::Specification.new do |spec|
spec.add_runtime_dependency 'tty-spinner', '~> 0.4'
spec.add_runtime_dependency 'tty-prompt', '~> 0.12'
spec.add_runtime_dependency 'json_pure', '~> 2.1.0'
spec.add_runtime_dependency 'tty-which', '~> 0.3.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

Addition of this dependency will require some work on the packaging side. Going to hold this for 0.5.0 so we can have time for packaging work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough :) I've opened a PR on the packaging repo to add this component for 0.5.0

@DavidS
Copy link
Contributor

DavidS commented Jul 24, 2017

tty-which is now in the packaging. Can we get this moving again? @puppetlabs/pdk

@scotje scotje dismissed bmjen’s stale review July 24, 2017 22:02

no longer relevant

@scotje scotje merged commit 17ec432 into puppetlabs:master Jul 24, 2017
@rodjek rodjek deleted the issue-137 branch July 27, 2017 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants