Skip to content
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

Deprecate the context argument #182

Merged
merged 2 commits into from
Feb 10, 2025
Merged

Deprecate the context argument #182

merged 2 commits into from
Feb 10, 2025

Conversation

haacked
Copy link
Contributor

@haacked haacked commented Feb 10, 2025

It's not used anywhere, a vestigial remnant of a time long ago.

This PR supercedes #54.

It's not used anywhere, a vestigial remnant of a time long ago.
@haacked haacked requested a review from mariusandra February 10, 2025 20:56
@haacked haacked mentioned this pull request Feb 10, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR deprecates and removes the unused 'context' parameter across the PostHog Python SDK, focusing on cleaning up legacy code.

  • Added deprecation warnings using warnings.warn() with DeprecationWarning in all relevant methods in posthog/__init__.py
  • Removed context parameter from command line arguments and function calls in simulator.py
  • Updated test cases in test_client.py to use named parameters and removed context-related assertions
  • Maintained backward compatibility while signaling future removal of the parameter across all affected files

4 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines 70 to 74
def set_once():
posthog.set_once(
options.distinct_id,
properties=json_hash(options.traits),
context=json_hash(options.context),
)
Copy link
Contributor

Choose a reason for hiding this comment

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

style: set_once is using traits for properties parameter which may be confusing - consider renaming the command line arg to match the function signature

Comment on lines 77 to 81
def set():
posthog.set(
options.distinct_id,
properties=json_hash(options.traits),
context=json_hash(options.context),
)
Copy link
Contributor

Choose a reason for hiding this comment

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

style: set is using traits for properties parameter which may be confusing - consider renaming the command line arg to match the function signature

@haacked haacked requested a review from a team February 10, 2025 23:22
Copy link
Member

@rafaeelaudibert rafaeelaudibert left a comment

Choose a reason for hiding this comment

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

🎉

@haacked haacked merged commit 4155080 into master Feb 10, 2025
2 checks passed
@haacked haacked deleted the haacked/deprecate-context branch February 10, 2025 23:26
@haacked haacked mentioned this pull request Feb 11, 2025
2 tasks
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.

2 participants