search

hybridgroup / gabba

Simple way to send server-side notifications to Google Analytics
Other
461 stars 54 forks source link

bugfix and some improvements #1

Closed mreinsch closed 13 years ago

mreinsch commented 13 years ago

Hi,

Please consider merging my changes.

Bugfix: all requests to GA didn't contain any query params Improvements: allow initialization from a Rack::Request

Cheers, Michael

deadprogram commented 13 years ago

Hi, Michael

Thanks for your patches. I did not accept them as submitted, in part because I did not want to switch to RSpec from MiniSpec/MiniTest. Also I like the concept of the Rack awareness, but I did not want to add all the specific dependencies.

That said, I looked over the commits, and took a bunch of the ideas in it. Take a look at my most recent commit, I credited you for the inspiration.

Thank you very much for your ideas and improvements!

Regards, Ron

mreinsch commented 13 years ago

Thanks for the credit. I'll then keep the fork (thinking of renaming it) because I want the logic to extract settings from a request encapsulated there (we use it to trigger ga requests for mobiles). I can understand that you don't want to introduce this dependency though. Cheers, Michael