Skip to content
This repository was archived by the owner on May 3, 2023. It is now read-only.

Conversation

@zyphlar
Copy link

@zyphlar zyphlar commented Aug 18, 2016

Fixes #36

This uses LookPath to check for dependency existence and
outputs a list of whichever dependencies are not yet installed.
Also outputs example apt/yum commands.

I don't see any test files or instructions so I'm assuming the testing portion of CONTRIBUTING.md is not applicable. This is the first time I've ever written anything in Go, so some manual verification is probably advised.

This uses LookPath to check for dependency existence and
outputs a list of whichever dependencies are not yet installed.
Also outputs example apt/yum commands.
@zyphlar zyphlar mentioned this pull request Aug 18, 2016
@kohlerm
Copy link

kohlerm commented Aug 24, 2016

Same problem here on Linux (RHEL 7 similiar to Centos 7), libxdo an wmctrl were not installed by default.
Actually just documenting that those binaries are needed would maybe make sense.
Update:
Sorry I missed running thyme dep. Anyway. I normally expect that dependencies are listed somewhere in the Readme.

@beyang
Copy link
Member

beyang commented Sep 5, 2016

Thanks for submitting this. I'd like to keep the dependency installation code simple, though, due to the fact that Thyme works across different windowing systems and OSes. As such, the external dependencies it relies on often are often not as straightforward to install. My concern here is that if we try to be too smart about it, it could make the dependency installation code more brittle to changes over time.

@beyang beyang closed this Sep 5, 2016
@beyang beyang mentioned this pull request Sep 6, 2016
@HalfVoxel
Copy link

Well. It doesn't make it more brittle at all. It simply checks if the dependencies are installed at all. Trying to run those dependencies later would definitely result in an (obscure) error, so why not have nicer errors? I see literally no downsides with this.
The 'dep' command seems to have caused a lot of confusion as well as there are several issues that were opened because they did not understand what the 'dep' command did.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants