-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
1.1.1 #10
1.1.1 #10
Conversation
tealium/__init__.py
Outdated
@@ -22,6 +22,14 @@ class Tealium(object): | |||
EVENT_TYPE_VIEW = "view" | |||
EVENT_TYPE_INTERACTION = "interaction" | |||
|
|||
# argument labels for track call. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets list both the EVENT_TYPEs and TRACKs alphabetically.
tealium/__init__.py
Outdated
@@ -75,6 +84,15 @@ def getUUIDandSave(self): | |||
afile.close() | |||
return UUID | |||
|
|||
def isValidEventType(self, eventType): | |||
eventTypeArray = [self.EVENT_TYPE_VIEW, self.EVENT_TYPE_DERIVED, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Break these up, one constant per line, and ordered alphabetically for clarity.
unittests.py
Outdated
@@ -34,7 +34,11 @@ def tealiumCallback(self, info, success, error=None): | |||
def test_trackEvent(self): | |||
t = Tealium('tealiummobile', 'demo', 'dev') | |||
d = {"foo": "bar"} | |||
t.trackEvent('title', t.EVENT_TYPE_VIEW, d, self.tealiumCallback) | |||
t.trackEvent(title="test", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I correct in assuming we can actually put these args in any order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
unittests.py
Outdated
data=d, | ||
eventType=Tealium.EVENT_TYPE_VIEW, | ||
tealiumCallback=self.tealiumCallback) | ||
|
||
t = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presuming this is needed to clear out the variable? If so, should be added to the other tests that assign a value to 't'
sample.py
Outdated
t.trackEvent(title="title", | ||
data=d, | ||
eventType=Tealium.EVENT_TYPE_VIEW, | ||
tealiumCallback=tealiumCallback) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Named arguments should only be for optional params.
tealium/__init__.py
Outdated
self.EVENT_TYPE_ACTIVITY, | ||
self.EVENT_TYPE_CONVERSION, | ||
self.EVENT_TYPE_INTERACTION] | ||
if (eventType in eventTypeArray): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be condensed into return eventType in eventTypeArray
tealium/__init__.py
Outdated
@@ -105,15 +126,16 @@ def enable(self): | |||
statusResult = False | |||
return statusResult | |||
|
|||
def trackEvent(self, title, eventType=None, data=None, tealiumCallback=None): | |||
def trackEvent(self, **arguments): | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the **
argument is the ellipsis operator ...
in other languages. This seems to contradict what the docstring says. Is there a purpose for it?
1.1.1