-
Notifications
You must be signed in to change notification settings - Fork 26
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
Add $set and $set_once support #23
Conversation
First time taking a stab at this library, so a thorough review would be appreciated. |
Ping @macobo should we move this forward? |
If we're doing |
Definitely agree with support |
@@ -125,7 +185,7 @@ def test_basic_page(self): | |||
|
|||
def test_basic_page_distinct_uuid(self): | |||
client = self.client | |||
distinct_id = uuid4() | |||
distinct_id = str(uuid4()) |
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.
Not sure how this worked before O.o
def set_once(): | ||
posthog.set_once( | ||
options.distinct_id, | ||
properties=json_hash(options.traits), | ||
context=json_hash(options.context), | ||
) | ||
|
||
|
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.
Just a nit, but might be good to add "set" here for completion as well.
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.
Gotcha, makes sense
Can you please still fix the black error... and then feel free to merge it into master :). |
Adds
$set
and$set_once
support to the library. Related to PostHog/posthog#2684.Further, makes things easier to test & run locally. Using
setup.py test
is deprecated, and so istest_requires
. Thus, I changed the test file names and added support for pytest so we can still test things. I didn't change any CI configuration, which hopefully gives confidence that the new and old testing methods are on par.If you're okay with this change, I can "upgrade"
ci.yaml
as well.