zevarito / mixpanel

Simple lib to track events in Mixpanel service. It can be used in any rack based framework.
MIT License
273 stars 84 forks source link

Code Revision #47

Closed sturgill closed 11 years ago

sturgill commented 11 years ago

As discussed, here's a quick refactor making use of Mixpanel::Tracker. I've tested it to work in JRuby, 1.8.7, REE, 1.9.2, and 1.9.3. The version number would just need to be updated to reflect the change (also, your recent change isn't part of this pull request).

zevarito commented 11 years ago

Hi @sturgill, thanks for submitting your work.

Before merge your changes I need to ask you to update your method definitions following the coding style already used in the gem. You are defining methods without include () around parameters, we only use that style if the method doesn't receive any paramaters. Thanks again.

sturgill commented 11 years ago

Done.

zevarito commented 11 years ago

@sturgill Merge is giving me some conflicts, I will fix those Tomorrow. Thanks.

zevarito commented 11 years ago

Hi @sturgill, if you are able to resolve the conflicts by merging master in your work, that will be really helpful to delivery a new version of the gem faster. I am with lot of client work this week and I am not sure where I will have time to make this. Thanks again.

sturgill commented 11 years ago

Done and pushed. All that you should need to do is add a line to the CHANGELOG and update the version in mixpanel.gemspec.

zevarito commented 11 years ago

Done and pushed new version of the Gem. Thanks.