make tinder gem an optional dependency #16

Merged
merged 1 commit into from Jan 28, 2013

Conversation

Projects
None yet
3 participants
Contributor

afeld commented Jan 20, 2013

We don't use Campfire, and having to install the tinder gem when using capistrano-helpers adds a lot of cruft. With this change, it won't be installed as part of the gem dependencies, but an error with instructions will be thrown if someone tries to use the Campfire notifier and tinder isn't available.

anatraj commented Jan 22, 2013

Hi Aiden,

This looks like a great addition; that has annoyed us in the past as well. I'll review this and your other pull requests by the end of the week. Thank you very much for the contributions!

On Jan 20, 2013, at 5:23 AM, Aidan Feldman notifications@github.com wrote:

We don't use Campfire, and having to install the tinder gem when using capistrano-helpers adds a lot of cruft. With this change, it won't be installed as part of the gem dependencies, but an error with instructions will be thrown if someone tries to use the Campfire notifier and tinder isn't available.

You can merge this Pull Request by running

git pull https://github.com/afeld/capistrano-helpers no-tinder
Or view, comment on, or merge it at:

#16

Commit Summary

make tinder gem an optional dependency
File Changes

M CHANGELOG.markdown (1)
M README.rdoc (6)
M Rakefile (3)
M capistrano-helpers.gemspec (21)
M lib/capistrano-helpers/campfire.rb (7)
Patch Links:

https://github.com/westarete/capistrano-helpers/pull/16.patch
https://github.com/westarete/capistrano-helpers/pull/16.diff

Scott Woods
West Arete
http://westarete.com
scott@westarete.com
814.753.4951

Contributor

afeld commented Jan 22, 2013

No problem! FYI I merged the three together in my master branch and am using that repo for @Jux in the meantime. Have done a few deploys to staging and production since then, using ding and growl... without issue, which I hope inspires confidence :-)

Campfire notification should still be tested in the positive case, btw.

Owner

woods commented Jan 28, 2013

This looks good and works well for me in a simple case. It exposes the fact that we need to clean up the gemspec file before the next release, but that's a separate issue; it shouldn't prevent this from being merged.

Thank you again for the contributions, Aidan!

woods added a commit that referenced this pull request Jan 28, 2013

Merge pull request #16 from afeld/no-tinder
make tinder gem an optional dependency

@woods woods merged commit 6814eb6 into westarete:master Jan 28, 2013

Contributor

afeld commented Jan 28, 2013

Cool - gemspec is very easy to regenerate through the rake task. Let me know when a new version is released!

@afeld afeld deleted the afeld:no-tinder branch Jan 28, 2013

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