Skip to content

Conversation

@jbwyme
Copy link

@jbwyme jbwyme commented Jun 20, 2013

No description provided.

mixpanel.py Outdated
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably make these properties of the class

@joeatwork
Copy link
Contributor

Killer start!

mixpanel.py Outdated

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be:

class Mixpanel(object):

to be a "new-style" python class

@jbwyme
Copy link
Author

jbwyme commented Jul 1, 2013

We should probably have a way for a user to register a distinct_id to be tracked with all track calls. We've done this in our other libraries via an identify(distinct_id) method that stores it off on the class instance. In fact in the PHP lib I just have a super_properties array member variable that holds a list of properties to pass with every call and identify simply sets $super_properties['distinct_id'] in that array.

test_mixpanel.py Outdated
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't exist anymore right?

mixpanel.py Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment block should go inside function, and prefixed underscore means internal so no need to say it

mixpanel.py Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment blocks should be indented to match code inside fn

amyquispe pushed a commit that referenced this pull request Jul 3, 2013
@amyquispe amyquispe merged commit e83927b into master Jul 3, 2013
@amyquispe amyquispe deleted the initial-lib branch July 3, 2013 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants