-
-
Notifications
You must be signed in to change notification settings - Fork 170
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 event tag to gtag for enhanced ecommerce google analytics #186
base: main
Are you sure you want to change the base?
Add event tag to gtag for enhanced ecommerce google analytics #186
Conversation
Codecov Report
@@ Coverage Diff @@
## master #186 +/- ##
==========================================
- Coverage 94.25% 93.85% -0.40%
==========================================
Files 29 29
Lines 1270 1285 +15
==========================================
+ Hits 1197 1206 +9
- Misses 73 79 +6
Continue to review full report at Codecov.
|
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.
Please add at least one test for the new functionality in the related unit test module.
From the test the expected outcome (the resulting JavaScript code) should be clear.
that's so nice Co-authored-by: Peter Bittner <[email protected]>
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.
Please rebase your PR and add a test for the business logic you added. Thank you!
try: | ||
dict(value) | ||
except ValueError: | ||
value = f"'{value}'" |
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.
This doesn't look like a valid pattern. The try
-block shouldn't be use for testing on a data type. And what's code in the except
-clause? I think this is an edge case of a "pythonic try-except" where the pattern doesn't fit.
It's not clear at first sight what we're trying to achieve. Maybe an if instanceof(value, dict)
would be better here, because it's more explicit for the use case.
as mentioned in #185 i modify google_analytics_gtag.py to get set event data in gtag.
hopefully be useful