jkrall / analytical

Gem for managing multiple analytics services in your rails app.
MIT License
380 stars 92 forks source link

KISS Metrics alias method #3

Closed scudco closed 13 years ago

scudco commented 13 years ago

I'm already using analytical, and I need to use the alias method for KISS Metrics. I didn't want to write a bunch of hacky javascript so I did my best to extend the analytical api to include alias.

jkrall commented 13 years ago

Great! Glad to pull this... makes total sense.

Before I do: Have you checked that a call to analytical.alias_identity is ignored by all other modules? I've been meaning to refactor a little and use a more duck-typing style so that we check respond_to?() before calling the method on each module, but I haven't had time to make that change yet.

Just want to make sure that if you have both Clicky and KISS modules running, that a call to analytical.alias_identity() doesn't trigger an exception in the Clicky module.

jkrall commented 13 years ago

Incidentally... if you feel like adding that respond_to?() check in this patch, it's pretty straightforward. I think it belongs in the process_queue_commands() method here: http://github.com/jkrall/analytical/blob/master/lib/analytical/base.rb#L43

Just collect either an empty string, or the result of the send(), depending on if the module responds to the queued method call.

scudco commented 13 years ago

Awesome! I'll look into the respond_to?() stuff.

I didn't dig too deep into the inner workings in the previous commits, but I did define an #alias_identity method in the base class. However, I totally agree that ensuring a class is going to respond to a method before calling it would make it easier for module specific methods like this.

scudco commented 13 years ago

Let me know if that looks right. All specs passing.

jkrall commented 13 years ago

Looks perfect! I'll pull it in now and push out a new release.