Skip to content
This repository was archived by the owner on May 17, 2024. It is now read-only.

add usage type tracking #346

Merged
merged 1 commit into from
Dec 12, 2022
Merged

add usage type tracking #346

merged 1 commit into from
Dec 12, 2022

Conversation

kylemcnair
Copy link
Contributor

@kylemcnair kylemcnair commented Dec 12, 2022

Summary
This PR adds tracking to differentiate diffs triggered by CLI vs Python API

Detail

  • adds set_usage_mode(str) to tracking.py
  • sets usage mode to "CLI" and "Python API" in __main__ and __init__ respectively
  • passes usage mode as event properties

I called it "usage mode", but am very very open to alternative naming conventions if something better communicates this property.

Testing
Example Python API diff:
Screen Shot 2022-12-12 at 9 03 50 AM

Example CLI diff
Screen Shot 2022-12-12 at 9 25 44 AM

Formatting

data-diff [track_cli_vs_python●] black data_diff/tracking.py                                            
All done! ✨ 🍰 ✨
1 file left unchanged.

@kylemcnair kylemcnair requested a review from erezsh December 12, 2022 17:26
@kylemcnair kylemcnair self-assigned this Dec 12, 2022
@kylemcnair kylemcnair requested a review from dlawin December 12, 2022 17:26
@erezsh
Copy link
Contributor

erezsh commented Dec 12, 2022

We can call it "entrypoint name".

Btw, just set the default value to "api". We only need change the value when running from main.

@kylemcnair
Copy link
Contributor Author

We can call it "entrypoint name".

Btw, just set the default value to "api". We only need change the value when running from main.

That makes sense, updated 👍

@erezsh
Copy link
Contributor

erezsh commented Dec 12, 2022

Great. Any chance you can flatten them into 1 commit? (using rebase)

Also, a tiny detail, better to change the str param to something else, since str is a built in type, and it might be a little confusing. You can rename it to name or even s.

@kylemcnair
Copy link
Contributor Author

Great. Any chance you can flatten them into 1 commit? (using rebase)

Also, a tiny detail, better to change the str param to something else, since str is a built in type, and it might be a little confusing. You can rename it to name or even s.

Done!

@erezsh erezsh merged commit 7abe5a5 into master Dec 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants