search

librato / statsd-cookbook

Chef cookbook to install Statsd
Apache License 2.0
26 stars 36 forks source link

simplifying the upstart script #15

Closed 5c077yP closed 10 years ago

5c077yP commented 10 years ago

Hey,

is there any specific reason why you're using the upstart script in combination with a start script ? This PR simplifies the upstart script. I adjusted the default recipe to not create the scripts folder, since there are no other script located here.

Btw. what is the 

env SL_NAME=statsd

in the previous upstart script for ?

mheffner commented 10 years ago

Looks good to me. SL_NAME=statsd was something we use to use for Silverline, but it should be removed now.

@johnclaus Any comments?

scalp42 commented 10 years ago

Any chance to get this rebased please ? @mheffner @johnclaus @5c077yP

mheffner commented 10 years ago

@scalp42 I think SL_NAME=... can still be removed?

scalp42 commented 10 years ago

Would be awesome if you guys could push a tag with the release number v2.x if this PR gets merged!

scalp42 commented 10 years ago

I'm gonna cherry-pick @5c077yP work and merge it in #22

scalp42 commented 10 years ago

I think we can close that one.

5c077yP commented 10 years ago

@scalp42 thanks for your work, short question i would like to split the install procedure from the configure procedure, e.g. sometimes i need to reconfigure the graphite host, but because npm runs (and sometimes it is not available) my chef runs fail. if i prepare a second PR can you tell me what you think about it ?

scalp42 commented 10 years ago

@5c077yP I've seen this before (same with rubygems or pypi) and we pretty much count these failures as "acceptable".

The only issue I see if that if you split the configure and install (fine with), it also means you'd have to change the run_list down the line to not include the install recipe anymore, as well as not updating anymore the statsd backends (as the npm execute is not run anymore).

But if the install recipe include the configure recipe by default and doesn't change the default behavior, why not? :+1:

Will also need to split the specs, I can help in case just ping me wherever.